[Partner Nodes] feat: ImageCompositor node with layer-state compositing, layer from bbox and Seedream Layer Separation node - #15317
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change adds the public 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@comfy_extras/compositor_blend.py`:
- Around line 276-289: Remove the obsolete non-linear composite path in the
mode-resolution flow. Since resolve_mode always creates EffectiveMode with
linear compositing, drop the composite_space field from EffectiveMode and
resolve_mode, then simplify the affected compositing function to directly call
run_composite without the conversion and concatenation logic.
In `@comfy_extras/nodes_compositor.py`:
- Around line 52-61: Reduce the work in input_fingerprints by hashing a
deterministic strided subsample of each tensor frame instead of converting and
hashing the full-resolution frame. Preserve the tensor shape in the digest and
ensure the subsampling remains stable so input changes can still invalidate
stale recipes.
- Around line 237-244: Clamp the layer opacity returned by the layer parsing
logic to the inclusive range [0, 1], matching the behavior of _parse_background.
Apply the clamp to the _number(entry, "opacity", 1.0) value before it is
consumed as cov by blend_composite, while preserving the existing default and
other layer fields.
- Around line 82-95: Update layout_bboxes to normalize each _bbox_entries entry
before calling boxes_from_input, because the current loop is feeding flattened
coordinates or dict entries into a parser that rejects them and returns empty
slots. Reuse the same parsing path for each supported entry shape, then take the
per-box result for each slot instead of catching ValueError and converting
everything to None. Keep _bbox_entries as the entry source and preserve the
existing slot-order behavior in layout_bboxes.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 610a4e5e-a06d-4ac0-b188-01c32e1b5c30
📒 Files selected for processing (4)
comfy_api/latest/_io.pycomfy_extras/compositor_blend.pycomfy_extras/nodes_compositor.pynodes.py
📜 Review details
⏰ Context from checks skipped due to timeout. (8)
- GitHub Check: test
- GitHub Check: Run Pylint
- GitHub Check: test (ubuntu-latest)
- GitHub Check: test (macos-latest)
- GitHub Check: test (windows-2022)
- GitHub Check: test (ubuntu-latest)
- GitHub Check: test (macos-latest)
- GitHub Check: test (windows-latest)
🧰 Additional context used
📓 Path-based instructions (7)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Keep changes small, direct, and limited to the narrowest necessary code path and smallest number of files.
Prefer practical fixes, minimal dependencies, and existing repository patterns; remove obsolete, dead, unreachable, or unused code.
Preserve existing APIs, node names, model-loading behavior, file layout, and workflow compatibility unless replacement is explicitly intended.
Core ComfyUI must not add outbound internet requests, telemetry, tracking, reporting, remote configuration, or background network activity. User-authorized model downloads are limited to the requested artifact and must exclude telemetry and unrelated metadata.
Files:
nodes.pycomfy_api/latest/_io.pycomfy_extras/compositor_blend.pycomfy_extras/nodes_compositor.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Keep state and capability flags on the object that owns the behavior. Prefer explicit parent-owned attributes over probing child objects withgetattr; use child checks only when the child owns the delegated behavior.
Preserve shared method signatures, argument order, return shapes, side effects, and error behavior unless every affected caller and interface is intentionally updated.
Do not add unused compatibility parameters, flags, attributes, constructor options, or model-specific options to shared helpers; keep one-off behavior at the integration boundary.
Normalize third-party return conventions at integration boundaries so core code receives the expected type and shape; avoid undocumented caller-side unwrapping.
Do not addtorch.no_grad,torch.inference_mode, or inference-mode wrappers. Do not add model freeze/unfreeze toggles; only disable globally enabled inference mode when a training path requires gradients.
Remove inference-only training behavior such as dropout while preserving checkpoint and state-dict compatibility; usenn.Identitywhen deleting a module would alter keys or ordering.
Keep imports at module scope except established optional-backend probes or imports required to avoid cycles; avoid unnecessarytry/exceptblocks and use specific exceptions with useful fallbacks.
Do not add workarounds for unsupported library versions, especially PyTorch exception-and-float-cast retries, unless a comment names the exact versions still requiring them.
Let unsupported model formats, invalid quantization metadata, and bad states fail with clear errors instead of silently degrading output.
Match local style, keep comments sparse and useful, and remove comments that merely restate obvious code.
Treat dtype, device placement, VRAM use, and offloading as correctness concerns across CPU, CUDA, ROCm, MPS, DirectML, XPU, NPU, and low-VRAM environments.
Prefer existing ComfyUI and Comfy Kitchen operations, quantization helpers, cast/offload helpe...
Files:
nodes.pycomfy_api/latest/_io.pycomfy_extras/compositor_blend.pycomfy_extras/nodes_compositor.py
**/*.{py,json}
📄 CodeRabbit inference engine (AGENTS.md)
Treat legacy combo,
io.Combo, andio.DynamicCombovalues affecting filesystem access as untrusted; revalidate them at load/save boundaries withfolder_paths, containment checks, or fixed allowlists.
Files:
nodes.pycomfy_api/latest/_io.pycomfy_extras/compositor_blend.pycomfy_extras/nodes_compositor.py
**/*.{py,md,txt,json}
📄 CodeRabbit inference engine (AGENTS.md)
Keep warning and info messages short and actionable, remove noisy or misleading logging, and make documentation edits concise, factual, and tied to changed behavior.
Files:
nodes.pycomfy_api/latest/_io.pycomfy_extras/compositor_blend.pycomfy_extras/nodes_compositor.py
**
⚙️ CodeRabbit configuration file
**: IMPORTANT: Only comment on issues directly introduced by this PR's code changes.
Treat AGENTS.md as mandatory repository policy, not optional style guidance.
Flag PR changes that violate AGENTS.md even when the code is otherwise functional.
In particular, enforce architecture boundaries, dtype/device/memory rules,
interface contracts, import style, no unnecessary try/except blocks, no inline
imports, no outbound internet paths in core ComfyUI, and narrow scoped fixes.
Prefer direct findings over suggestions when a rule is violated. Only ignore
AGENTS.md when it clearly conflicts with a newer explicit maintainer instruction
in the PR.
Do NOT flag pre-existing issues in code that was merely moved, re-indented,
de-indented, or reformatted without logic changes. If code appears in the diff
only due to whitespace or structural reformatting (e.g., removing awith:block),
treat it as unchanged. Contributors should not feel obligated to address
pre-existing issues outside the scope of their contribution.
Files:
nodes.pycomfy_api/latest/_io.pycomfy_extras/compositor_blend.pycomfy_extras/nodes_compositor.py
nodes.py
⚙️ CodeRabbit configuration file
nodes.py: Core node definitions (2500+ lines). Focus on:
- Backward compatibility of NODE_CLASS_MAPPINGS
- Consistency of INPUT_TYPES return format
Files:
nodes.py
comfy_extras/**
⚙️ CodeRabbit configuration file
comfy_extras/**: Community-contributed extra nodes. Focus on:
- Consistency with node patterns (INPUT_TYPES, RETURN_TYPES, FUNCTION, CATEGORY)
- No breaking changes to existing node interfaces
Files:
comfy_extras/compositor_blend.pycomfy_extras/nodes_compositor.py
🧠 Learnings (6)
📚 Learning: 2026-02-21T14:01:41.482Z
Learnt from: pythongosssss
Repo: Comfy-Org/ComfyUI PR: 12555
File: comfy_extras/nodes_glsl.py:719-724
Timestamp: 2026-02-21T14:01:41.482Z
Learning: In PyOpenGL, bare Python scalars can be accepted for 1-element array parameters by NumberHandler. This means you can pass an int/float directly to OpenGL texture deletion (e.g., glDeleteTextures(tex)) without wrapping in a list. Verify function-specific expectations and ensure types match what the OpenGL call expects; use explicit lists only when the API requires an array.
Applied to files:
nodes.pycomfy_api/latest/_io.pycomfy_extras/compositor_blend.pycomfy_extras/nodes_compositor.py
📚 Learning: 2026-03-04T14:05:31.426Z
Learnt from: jtydhr88
Repo: Comfy-Org/ComfyUI PR: 12757
File: comfy_extras/nodes_custom_sampler.py:1069-1089
Timestamp: 2026-03-04T14:05:31.426Z
Learning: In the ComfyUI sampling pipeline, treat percent_to_sigma(0.0) as a sentinel value (999999999.9) that means starting from pure noise. This is consistent with BasicScheduler via calculate_sigmas. The SamplingPercentToSigma node’s return_actual_sigma flag differentiates this sentinel from sigma_max. Reviewers should not flag CurveToSigmas or similar nodes that rely on percent_to_sigma as bugs; downstream samplers are expected to handle the sentinel correctly. When reviewing related sampling-related code, assume this sentinel semantics unless there is explicit handling for a real sigma_max.
Applied to files:
comfy_extras/compositor_blend.pycomfy_extras/nodes_compositor.py
📚 Learning: 2026-04-04T13:29:15.653Z
Learnt from: kijai
Repo: Comfy-Org/ComfyUI PR: 13258
File: comfy_extras/nodes_frame_interpolation.py:151-189
Timestamp: 2026-04-04T13:29:15.653Z
Learning: In this ComfyUI codebase, node `execute()` inference is already run under a global `torch.inference_mode()` context established in the execution engine (e.g., `execution.py` around line ~732). During review, avoid recommending changes that wrap node inference loops in `torch.inference_mode()`—it is already applied, so such suggestions are likely redundant.
Applied to files:
comfy_extras/compositor_blend.pycomfy_extras/nodes_compositor.py
📚 Learning: 2026-05-09T18:40:40.199Z
Learnt from: kijai
Repo: Comfy-Org/ComfyUI PR: 13813
File: comfy_extras/nodes_wandancer.py:868-872
Timestamp: 2026-05-09T18:40:40.199Z
Learning: When building video/temporal decoding nodes that call ComfyUI’s VAE.decode (comfy/sd.py), leverage VAE.decode’s existing VRAM-aware chunking along dim 0. Reshape or transpose the latent so the temporal dimension T is folded into dim 0 (e.g., transform a latent of shape [B, T, C, H, W] into [B*T, C, H, W] before calling vae.decode). This lets VAE.decode do chunked decoding without needing an explicit per-frame loop inside the node itself.
Applied to files:
comfy_extras/compositor_blend.pycomfy_extras/nodes_compositor.py
📚 Learning: 2026-05-20T00:10:14.673Z
Learnt from: Pauan
Repo: Comfy-Org/ComfyUI PR: 13997
File: comfy_extras/nodes_string.py:12-25
Timestamp: 2026-05-20T00:10:14.673Z
Learning: In the ComfyUI `comfy_extras/` codebase, some nodes intentionally ship with a default input string that references parameters that may not yet be connected. If the default would raise a `KeyError` (e.g., examples like `MathExpression` default `a + b`, or `StringFormat` default `{a}` with `min=0` and autogrow inputs), treat it as an intentional “hint default” UX pattern, not a bug. During review, do not flag this behavior or recommend changing `min` to `1` or altering the default to an empty string solely to avoid the `KeyError`.
Applied to files:
comfy_extras/compositor_blend.pycomfy_extras/nodes_compositor.py
📚 Learning: 2026-07-26T18:37:44.213Z
Learnt from: jtydhr88
Repo: Comfy-Org/ComfyUI PR: 15090
File: comfy_extras/nodes_video.py:230-235
Timestamp: 2026-07-26T18:37:44.213Z
Learning: In ComfyUI node implementations under `comfy_extras`, do not add regular widget/prompt inputs to `fingerprint_inputs` if they are already included in the node cache signature via `comfy_execution/caching.py:get_immediate_node_signature` (it records every non-link prompt input as `(key, inputs[key])`). Reserve `fingerprint_inputs` only for out-of-band state that can change without changing the prompt inputs (e.g., the selected source file’s modification time). For example, inputs like `LoadVideo.edit` should not be redundantly added to `fingerprint_inputs`; use it only for things not represented in prompt inputs.
Applied to files:
comfy_extras/compositor_blend.pycomfy_extras/nodes_compositor.py
🔇 Additional comments (12)
comfy_api/latest/_io.py (2)
2424-2424: LGTM!
860-865: 📐 Maintainability & Code QualityNo change needed. The forwarded argument order matches
WidgetInput.comfy_extras/compositor_blend.py (3)
15-37: LGTM!
40-258: LGTM!
292-308: LGTM!comfy_extras/nodes_compositor.py (6)
22-49: LGTM!
98-130: LGTM!
133-222: LGTM!
251-325: LGTM!
328-407: LGTM!
245-245: 🎯 Functional CorrectnessConfirm the compositor property-panel rotation unit contract before changing this code.
savedstate initializesrotationto0,parse_layer_statepreserves the incoming value, and compositor math reads it as radians while the image is rotated once through-math.degrees(params["rotation"]). If the property panel stores degrees, non-zero rotations render at the wrong angle.nodes.py (1)
2495-2495: LGTM!
| def define_schema(cls): | ||
| return io.Schema( | ||
| node_id="ImageCompositor", | ||
| display_name="Image Compositor", |
There was a problem hiding this comment.
| display_name="Image Compositor", | |
| display_name="Create Layered Image", |
| min=1, | ||
| max=50, | ||
| ), | ||
| tooltip="Layers to composite. The first input is the bottom layer; each subsequent input is stacked above the previous one.", |
There was a problem hiding this comment.
| tooltip="Layers to composite. The first input is the bottom layer; each subsequent input is stacked above the previous one.", | |
| tooltip="Layers to composite. The first image is the back layer; each subsequent image is stacked above the previous one.", |
| "bboxes", | ||
| [io.BoundingBox, io.Array, io.String], | ||
| optional=True, | ||
| tooltip="Optional initial layout: bounding boxes, elements, or a JSON string, index-aligned with the image inputs (bboxes[0] places image_0). Inputs without a box keep their natural size at the origin. A saved compositor recipe that matches the current inputs takes priority.", |
There was a problem hiding this comment.
I would simplify:
| tooltip="Optional initial layout: bounding boxes, elements, or a JSON string, index-aligned with the image inputs (bboxes[0] places image_0). Inputs without a box keep their natural size at the origin. A saved compositor recipe that matches the current inputs takes priority.", | |
| tooltip="Optional bounding boxes to initialize the layout, index-aligned with the image inputs (bboxes[0] places image_0). Images without a bounding box keep their natural size at the origin. A saved composition that matches the current set of inputs takes priority.", |
| ), | ||
| io.Compositor.Input( | ||
| "compositor", | ||
| tooltip="Layer recipe saved by the compositor editor, replayed over the current inputs", |
There was a problem hiding this comment.
| tooltip="Layer recipe saved by the compositor editor, replayed over the current inputs", | |
| tooltip="Layered composition saved by the compositor editor.", |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
comfy_api/latest/_io.py (1)
865-866: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winHandle the empty
Compositordefault before reading required fields.
parse_layer_staterejects{}before it can be normalized as an empty state, butCompositor.Inputemits{}when no default is provided. Normalize{}at the parser boundary to the same empty-state shape asstate_from_bboxes, or change theCompositor.Inputdefault; otherwise missing inputs cannot update cached state metadata consistently.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@comfy_api/latest/_io.py` around lines 865 - 866, Update the parse_layer_state boundary to accept an empty Compositor default ({}) and normalize it to the same empty-state shape produced by state_from_bboxes before validating required fields. Preserve existing validation for non-empty states and ensure Compositor.Input’s no-default output can update cached state metadata consistently.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@comfy_extras/nodes_compositor.py`:
- Around line 75-79: Update canvas_size and the composite_from_state flow to
enforce the shared maximum canvas dimension and pixel-count policy before
allocating the float32 RGBA fallback canvas. Validate the computed dimensions
and reject oversized inputs with an actionable error, ensuring validation occurs
before fallback state creation or any large allocation.
- Around line 219-227: Update _parse_order to validate order against the current
layer count, accepting only a non-empty permutation containing every layer index
exactly once; reject partial, duplicate, out-of-range, and otherwise invalid
values. Ensure composite_from_state falls back to natural input order whenever
this validation fails.
---
Outside diff comments:
In `@comfy_api/latest/_io.py`:
- Around line 865-866: Update the parse_layer_state boundary to accept an empty
Compositor default ({}) and normalize it to the same empty-state shape produced
by state_from_bboxes before validating required fields. Preserve existing
validation for non-empty states and ensure Compositor.Input’s no-default output
can update cached state metadata consistently.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 69844840-781b-4125-a56f-8ac5aa02410c
📒 Files selected for processing (2)
comfy_api/latest/_io.pycomfy_extras/nodes_compositor.py
📜 Review details
⏰ Context from checks skipped due to timeout. (8)
- GitHub Check: test (macos-latest)
- GitHub Check: test (ubuntu-latest)
- GitHub Check: test (windows-2022)
- GitHub Check: Run Pylint
- GitHub Check: test (windows-latest)
- GitHub Check: test (macos-latest)
- GitHub Check: test (ubuntu-latest)
- GitHub Check: Run Pylint
🧰 Additional context used
📓 Path-based instructions (6)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Keep changes small, direct, and limited to the narrowest necessary code path and smallest number of files.
Prefer practical fixes, minimal dependencies, and existing repository patterns; remove obsolete, dead, unreachable, or unused code.
Preserve existing APIs, node names, model-loading behavior, file layout, and workflow compatibility unless replacement is explicitly intended.
Core ComfyUI must not add outbound internet requests, telemetry, tracking, reporting, remote configuration, or background network activity. User-authorized model downloads are limited to the requested artifact and must exclude telemetry and unrelated metadata.
Files:
comfy_api/latest/_io.pycomfy_extras/nodes_compositor.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Keep state and capability flags on the object that owns the behavior. Prefer explicit parent-owned attributes over probing child objects withgetattr; use child checks only when the child owns the delegated behavior.
Preserve shared method signatures, argument order, return shapes, side effects, and error behavior unless every affected caller and interface is intentionally updated.
Do not add unused compatibility parameters, flags, attributes, constructor options, or model-specific options to shared helpers; keep one-off behavior at the integration boundary.
Normalize third-party return conventions at integration boundaries so core code receives the expected type and shape; avoid undocumented caller-side unwrapping.
Do not addtorch.no_grad,torch.inference_mode, or inference-mode wrappers. Do not add model freeze/unfreeze toggles; only disable globally enabled inference mode when a training path requires gradients.
Remove inference-only training behavior such as dropout while preserving checkpoint and state-dict compatibility; usenn.Identitywhen deleting a module would alter keys or ordering.
Keep imports at module scope except established optional-backend probes or imports required to avoid cycles; avoid unnecessarytry/exceptblocks and use specific exceptions with useful fallbacks.
Do not add workarounds for unsupported library versions, especially PyTorch exception-and-float-cast retries, unless a comment names the exact versions still requiring them.
Let unsupported model formats, invalid quantization metadata, and bad states fail with clear errors instead of silently degrading output.
Match local style, keep comments sparse and useful, and remove comments that merely restate obvious code.
Treat dtype, device placement, VRAM use, and offloading as correctness concerns across CPU, CUDA, ROCm, MPS, DirectML, XPU, NPU, and low-VRAM environments.
Prefer existing ComfyUI and Comfy Kitchen operations, quantization helpers, cast/offload helpe...
Files:
comfy_api/latest/_io.pycomfy_extras/nodes_compositor.py
**/*.{py,json}
📄 CodeRabbit inference engine (AGENTS.md)
Treat legacy combo,
io.Combo, andio.DynamicCombovalues affecting filesystem access as untrusted; revalidate them at load/save boundaries withfolder_paths, containment checks, or fixed allowlists.
Files:
comfy_api/latest/_io.pycomfy_extras/nodes_compositor.py
**/*.{py,md,txt,json}
📄 CodeRabbit inference engine (AGENTS.md)
Keep warning and info messages short and actionable, remove noisy or misleading logging, and make documentation edits concise, factual, and tied to changed behavior.
Files:
comfy_api/latest/_io.pycomfy_extras/nodes_compositor.py
**
⚙️ CodeRabbit configuration file
**: IMPORTANT: Only comment on issues directly introduced by this PR's code changes.
Treat AGENTS.md as mandatory repository policy, not optional style guidance.
Flag PR changes that violate AGENTS.md even when the code is otherwise functional.
In particular, enforce architecture boundaries, dtype/device/memory rules,
interface contracts, import style, no unnecessary try/except blocks, no inline
imports, no outbound internet paths in core ComfyUI, and narrow scoped fixes.
Prefer direct findings over suggestions when a rule is violated. Only ignore
AGENTS.md when it clearly conflicts with a newer explicit maintainer instruction
in the PR.
Do NOT flag pre-existing issues in code that was merely moved, re-indented,
de-indented, or reformatted without logic changes. If code appears in the diff
only due to whitespace or structural reformatting (e.g., removing awith:block),
treat it as unchanged. Contributors should not feel obligated to address
pre-existing issues outside the scope of their contribution.
Files:
comfy_api/latest/_io.pycomfy_extras/nodes_compositor.py
comfy_extras/**
⚙️ CodeRabbit configuration file
comfy_extras/**: Community-contributed extra nodes. Focus on:
- Consistency with node patterns (INPUT_TYPES, RETURN_TYPES, FUNCTION, CATEGORY)
- No breaking changes to existing node interfaces
Files:
comfy_extras/nodes_compositor.py
🧠 Learnings (6)
📚 Learning: 2026-02-21T14:01:41.482Z
Learnt from: pythongosssss
Repo: Comfy-Org/ComfyUI PR: 12555
File: comfy_extras/nodes_glsl.py:719-724
Timestamp: 2026-02-21T14:01:41.482Z
Learning: In PyOpenGL, bare Python scalars can be accepted for 1-element array parameters by NumberHandler. This means you can pass an int/float directly to OpenGL texture deletion (e.g., glDeleteTextures(tex)) without wrapping in a list. Verify function-specific expectations and ensure types match what the OpenGL call expects; use explicit lists only when the API requires an array.
Applied to files:
comfy_api/latest/_io.pycomfy_extras/nodes_compositor.py
📚 Learning: 2026-03-04T14:05:31.426Z
Learnt from: jtydhr88
Repo: Comfy-Org/ComfyUI PR: 12757
File: comfy_extras/nodes_custom_sampler.py:1069-1089
Timestamp: 2026-03-04T14:05:31.426Z
Learning: In the ComfyUI sampling pipeline, treat percent_to_sigma(0.0) as a sentinel value (999999999.9) that means starting from pure noise. This is consistent with BasicScheduler via calculate_sigmas. The SamplingPercentToSigma node’s return_actual_sigma flag differentiates this sentinel from sigma_max. Reviewers should not flag CurveToSigmas or similar nodes that rely on percent_to_sigma as bugs; downstream samplers are expected to handle the sentinel correctly. When reviewing related sampling-related code, assume this sentinel semantics unless there is explicit handling for a real sigma_max.
Applied to files:
comfy_extras/nodes_compositor.py
📚 Learning: 2026-04-04T13:29:15.653Z
Learnt from: kijai
Repo: Comfy-Org/ComfyUI PR: 13258
File: comfy_extras/nodes_frame_interpolation.py:151-189
Timestamp: 2026-04-04T13:29:15.653Z
Learning: In this ComfyUI codebase, node `execute()` inference is already run under a global `torch.inference_mode()` context established in the execution engine (e.g., `execution.py` around line ~732). During review, avoid recommending changes that wrap node inference loops in `torch.inference_mode()`—it is already applied, so such suggestions are likely redundant.
Applied to files:
comfy_extras/nodes_compositor.py
📚 Learning: 2026-05-09T18:40:40.199Z
Learnt from: kijai
Repo: Comfy-Org/ComfyUI PR: 13813
File: comfy_extras/nodes_wandancer.py:868-872
Timestamp: 2026-05-09T18:40:40.199Z
Learning: When building video/temporal decoding nodes that call ComfyUI’s VAE.decode (comfy/sd.py), leverage VAE.decode’s existing VRAM-aware chunking along dim 0. Reshape or transpose the latent so the temporal dimension T is folded into dim 0 (e.g., transform a latent of shape [B, T, C, H, W] into [B*T, C, H, W] before calling vae.decode). This lets VAE.decode do chunked decoding without needing an explicit per-frame loop inside the node itself.
Applied to files:
comfy_extras/nodes_compositor.py
📚 Learning: 2026-05-20T00:10:14.673Z
Learnt from: Pauan
Repo: Comfy-Org/ComfyUI PR: 13997
File: comfy_extras/nodes_string.py:12-25
Timestamp: 2026-05-20T00:10:14.673Z
Learning: In the ComfyUI `comfy_extras/` codebase, some nodes intentionally ship with a default input string that references parameters that may not yet be connected. If the default would raise a `KeyError` (e.g., examples like `MathExpression` default `a + b`, or `StringFormat` default `{a}` with `min=0` and autogrow inputs), treat it as an intentional “hint default” UX pattern, not a bug. During review, do not flag this behavior or recommend changing `min` to `1` or altering the default to an empty string solely to avoid the `KeyError`.
Applied to files:
comfy_extras/nodes_compositor.py
📚 Learning: 2026-07-26T18:37:44.213Z
Learnt from: jtydhr88
Repo: Comfy-Org/ComfyUI PR: 15090
File: comfy_extras/nodes_video.py:230-235
Timestamp: 2026-07-26T18:37:44.213Z
Learning: In ComfyUI node implementations under `comfy_extras`, do not add regular widget/prompt inputs to `fingerprint_inputs` if they are already included in the node cache signature via `comfy_execution/caching.py:get_immediate_node_signature` (it records every non-link prompt input as `(key, inputs[key])`). Reserve `fingerprint_inputs` only for out-of-band state that can change without changing the prompt inputs (e.g., the selected source file’s modification time). For example, inputs like `LoadVideo.edit` should not be redundantly added to `fingerprint_inputs`; use it only for things not represented in prompt inputs.
Applied to files:
comfy_extras/nodes_compositor.py
🔇 Additional comments (2)
comfy_api/latest/_io.py (2)
851-860: LGTM!
2425-2425: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@comfy_extras/nodes_compositor.py`:
- Around line 220-228: Update parse_layer_state to reject state when the parsed
layers and inputs collections have different lengths, before
composite_from_state replays them. Ensure only equal-length layers and inputs
proceed, preserving the existing stale-state fallback for mismatched recipes.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bde9dcea-45f9-44eb-a629-96f42042ab72
📒 Files selected for processing (2)
comfy_extras/compositor_blend.pycomfy_extras/nodes_compositor.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Keep changes small, direct, and limited to the narrowest necessary code path and smallest number of files.
Prefer practical fixes, minimal dependencies, and existing repository patterns; remove obsolete, dead, unreachable, or unused code.
Preserve existing APIs, node names, model-loading behavior, file layout, and workflow compatibility unless replacement is explicitly intended.
Core ComfyUI must not add outbound internet requests, telemetry, tracking, reporting, remote configuration, or background network activity. User-authorized model downloads are limited to the requested artifact and must exclude telemetry and unrelated metadata.
Files:
comfy_extras/compositor_blend.pycomfy_extras/nodes_compositor.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Keep state and capability flags on the object that owns the behavior. Prefer explicit parent-owned attributes over probing child objects withgetattr; use child checks only when the child owns the delegated behavior.
Preserve shared method signatures, argument order, return shapes, side effects, and error behavior unless every affected caller and interface is intentionally updated.
Do not add unused compatibility parameters, flags, attributes, constructor options, or model-specific options to shared helpers; keep one-off behavior at the integration boundary.
Normalize third-party return conventions at integration boundaries so core code receives the expected type and shape; avoid undocumented caller-side unwrapping.
Do not addtorch.no_grad,torch.inference_mode, or inference-mode wrappers. Do not add model freeze/unfreeze toggles; only disable globally enabled inference mode when a training path requires gradients.
Remove inference-only training behavior such as dropout while preserving checkpoint and state-dict compatibility; usenn.Identitywhen deleting a module would alter keys or ordering.
Keep imports at module scope except established optional-backend probes or imports required to avoid cycles; avoid unnecessarytry/exceptblocks and use specific exceptions with useful fallbacks.
Do not add workarounds for unsupported library versions, especially PyTorch exception-and-float-cast retries, unless a comment names the exact versions still requiring them.
Let unsupported model formats, invalid quantization metadata, and bad states fail with clear errors instead of silently degrading output.
Match local style, keep comments sparse and useful, and remove comments that merely restate obvious code.
Treat dtype, device placement, VRAM use, and offloading as correctness concerns across CPU, CUDA, ROCm, MPS, DirectML, XPU, NPU, and low-VRAM environments.
Prefer existing ComfyUI and Comfy Kitchen operations, quantization helpers, cast/offload helpe...
Files:
comfy_extras/compositor_blend.pycomfy_extras/nodes_compositor.py
**/*.{py,json}
📄 CodeRabbit inference engine (AGENTS.md)
Treat legacy combo,
io.Combo, andio.DynamicCombovalues affecting filesystem access as untrusted; revalidate them at load/save boundaries withfolder_paths, containment checks, or fixed allowlists.
Files:
comfy_extras/compositor_blend.pycomfy_extras/nodes_compositor.py
**/*.{py,md,txt,json}
📄 CodeRabbit inference engine (AGENTS.md)
Keep warning and info messages short and actionable, remove noisy or misleading logging, and make documentation edits concise, factual, and tied to changed behavior.
Files:
comfy_extras/compositor_blend.pycomfy_extras/nodes_compositor.py
**
⚙️ CodeRabbit configuration file
**: IMPORTANT: Only comment on issues directly introduced by this PR's code changes.
Treat AGENTS.md as mandatory repository policy, not optional style guidance.
Flag PR changes that violate AGENTS.md even when the code is otherwise functional.
In particular, enforce architecture boundaries, dtype/device/memory rules,
interface contracts, import style, no unnecessary try/except blocks, no inline
imports, no outbound internet paths in core ComfyUI, and narrow scoped fixes.
Prefer direct findings over suggestions when a rule is violated. Only ignore
AGENTS.md when it clearly conflicts with a newer explicit maintainer instruction
in the PR.
Do NOT flag pre-existing issues in code that was merely moved, re-indented,
de-indented, or reformatted without logic changes. If code appears in the diff
only due to whitespace or structural reformatting (e.g., removing awith:block),
treat it as unchanged. Contributors should not feel obligated to address
pre-existing issues outside the scope of their contribution.
Files:
comfy_extras/compositor_blend.pycomfy_extras/nodes_compositor.py
comfy_extras/**
⚙️ CodeRabbit configuration file
comfy_extras/**: Community-contributed extra nodes. Focus on:
- Consistency with node patterns (INPUT_TYPES, RETURN_TYPES, FUNCTION, CATEGORY)
- No breaking changes to existing node interfaces
Files:
comfy_extras/compositor_blend.pycomfy_extras/nodes_compositor.py
🧠 Learnings (6)
📚 Learning: 2026-02-21T14:01:41.482Z
Learnt from: pythongosssss
Repo: Comfy-Org/ComfyUI PR: 12555
File: comfy_extras/nodes_glsl.py:719-724
Timestamp: 2026-02-21T14:01:41.482Z
Learning: In PyOpenGL, bare Python scalars can be accepted for 1-element array parameters by NumberHandler. This means you can pass an int/float directly to OpenGL texture deletion (e.g., glDeleteTextures(tex)) without wrapping in a list. Verify function-specific expectations and ensure types match what the OpenGL call expects; use explicit lists only when the API requires an array.
Applied to files:
comfy_extras/compositor_blend.pycomfy_extras/nodes_compositor.py
📚 Learning: 2026-03-04T14:05:31.426Z
Learnt from: jtydhr88
Repo: Comfy-Org/ComfyUI PR: 12757
File: comfy_extras/nodes_custom_sampler.py:1069-1089
Timestamp: 2026-03-04T14:05:31.426Z
Learning: In the ComfyUI sampling pipeline, treat percent_to_sigma(0.0) as a sentinel value (999999999.9) that means starting from pure noise. This is consistent with BasicScheduler via calculate_sigmas. The SamplingPercentToSigma node’s return_actual_sigma flag differentiates this sentinel from sigma_max. Reviewers should not flag CurveToSigmas or similar nodes that rely on percent_to_sigma as bugs; downstream samplers are expected to handle the sentinel correctly. When reviewing related sampling-related code, assume this sentinel semantics unless there is explicit handling for a real sigma_max.
Applied to files:
comfy_extras/compositor_blend.pycomfy_extras/nodes_compositor.py
📚 Learning: 2026-04-04T13:29:15.653Z
Learnt from: kijai
Repo: Comfy-Org/ComfyUI PR: 13258
File: comfy_extras/nodes_frame_interpolation.py:151-189
Timestamp: 2026-04-04T13:29:15.653Z
Learning: In this ComfyUI codebase, node `execute()` inference is already run under a global `torch.inference_mode()` context established in the execution engine (e.g., `execution.py` around line ~732). During review, avoid recommending changes that wrap node inference loops in `torch.inference_mode()`—it is already applied, so such suggestions are likely redundant.
Applied to files:
comfy_extras/compositor_blend.pycomfy_extras/nodes_compositor.py
📚 Learning: 2026-05-09T18:40:40.199Z
Learnt from: kijai
Repo: Comfy-Org/ComfyUI PR: 13813
File: comfy_extras/nodes_wandancer.py:868-872
Timestamp: 2026-05-09T18:40:40.199Z
Learning: When building video/temporal decoding nodes that call ComfyUI’s VAE.decode (comfy/sd.py), leverage VAE.decode’s existing VRAM-aware chunking along dim 0. Reshape or transpose the latent so the temporal dimension T is folded into dim 0 (e.g., transform a latent of shape [B, T, C, H, W] into [B*T, C, H, W] before calling vae.decode). This lets VAE.decode do chunked decoding without needing an explicit per-frame loop inside the node itself.
Applied to files:
comfy_extras/compositor_blend.pycomfy_extras/nodes_compositor.py
📚 Learning: 2026-05-20T00:10:14.673Z
Learnt from: Pauan
Repo: Comfy-Org/ComfyUI PR: 13997
File: comfy_extras/nodes_string.py:12-25
Timestamp: 2026-05-20T00:10:14.673Z
Learning: In the ComfyUI `comfy_extras/` codebase, some nodes intentionally ship with a default input string that references parameters that may not yet be connected. If the default would raise a `KeyError` (e.g., examples like `MathExpression` default `a + b`, or `StringFormat` default `{a}` with `min=0` and autogrow inputs), treat it as an intentional “hint default” UX pattern, not a bug. During review, do not flag this behavior or recommend changing `min` to `1` or altering the default to an empty string solely to avoid the `KeyError`.
Applied to files:
comfy_extras/compositor_blend.pycomfy_extras/nodes_compositor.py
📚 Learning: 2026-07-26T18:37:44.213Z
Learnt from: jtydhr88
Repo: Comfy-Org/ComfyUI PR: 15090
File: comfy_extras/nodes_video.py:230-235
Timestamp: 2026-07-26T18:37:44.213Z
Learning: In ComfyUI node implementations under `comfy_extras`, do not add regular widget/prompt inputs to `fingerprint_inputs` if they are already included in the node cache signature via `comfy_execution/caching.py:get_immediate_node_signature` (it records every non-link prompt input as `(key, inputs[key])`). Reserve `fingerprint_inputs` only for out-of-band state that can change without changing the prompt inputs (e.g., the selected source file’s modification time). For example, inputs like `LoadVideo.edit` should not be redundantly added to `fingerprint_inputs`; use it only for things not represented in prompt inputs.
Applied to files:
comfy_extras/compositor_blend.pycomfy_extras/nodes_compositor.py
🔇 Additional comments (1)
comfy_extras/compositor_blend.py (1)
274-274: LGTM!
| "canvas": canvas_size(tensors), | ||
| "layers": layers, | ||
| "inputs": None, | ||
| "background": {"color": "#ffffff", "opacity": 1.0, "visible": True}, |
There was a problem hiding this comment.
Do we need to have a white matte? Consider graph-only path
There was a problem hiding this comment.
I will make it as transparent as default
There was a problem hiding this comment.
Confirmed landed, thanks - checked it at 3b3c61dd rather than taking it on trust.
state_from_items now emits {"color": "#ffffff", "opacity": 1.0, "visible": False} (comfy_extras/nodes_compositor.py:170), composite_from_state:374 skips the fill when visible is falsy, and composite_outputs:411-417 hands back RGBA plus the inverted MASK when the result is not opaque. So the graph-only path emits real transparency now, which was the case I was worried about.
The replay path agrees too, which I did not expect to have to check: the frontend's DEFAULT_BACKGROUND_ENTRY in compositorLayerState.ts is also visible: false, so a composition saved from the editor without an explicit background does not reintroduce the matte. Both paths line up.
And tests-unit/comfy_extras_test/compositor_node_test.py pins both halves (test_default_layout_background_is_hidden, test_uncovered_canvas_stays_transparent), so it will not drift back. Nothing further from me on this one.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
comfy_extras/nodes_compositor.py (1)
426-429: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake
compositoroptional.The implementation supports absent compositor state through
parse_layer_stateand the fallback path at Lines 455-465. Mark this state input as optional so the schema matches its execution contract.As per path instructions, AGENTS.md requires genuinely optional state inputs to be optional.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@comfy_extras/nodes_compositor.py` around lines 426 - 429, Update the compositor input declaration in the Compositor node’s input schema to mark “compositor” as optional, while preserving its existing tooltip and execution behavior through parse_layer_state and the fallback path.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@comfy_extras/nodes_compositor.py`:
- Around line 411-414: Replace the single required io.Image.Input("image")
declaration with io.Autogrow configured with a minimum of zero, exposing the
intended image_0..49-style variable inputs and allowing no inputs. Update
execute to process autogrow values by input slot first and then flatten each
slot’s frames, preserving the existing empty-input fallback.
---
Outside diff comments:
In `@comfy_extras/nodes_compositor.py`:
- Around line 426-429: Update the compositor input declaration in the Compositor
node’s input schema to mark “compositor” as optional, while preserving its
existing tooltip and execution behavior through parse_layer_state and the
fallback path.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8e184d3e-79e8-4bcd-aed6-1e5a45f98b8f
📒 Files selected for processing (1)
comfy_extras/nodes_compositor.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Keep changes small, direct, and limited to the narrowest necessary code path and smallest number of files.
Prefer practical fixes, minimal dependencies, and existing repository patterns; remove obsolete, dead, unreachable, or unused code.
Preserve existing APIs, node names, model-loading behavior, file layout, and workflow compatibility unless replacement is explicitly intended.
Core ComfyUI must not add outbound internet requests, telemetry, tracking, reporting, remote configuration, or background network activity. User-authorized model downloads are limited to the requested artifact and must exclude telemetry and unrelated metadata.
Files:
comfy_extras/nodes_compositor.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Keep state and capability flags on the object that owns the behavior. Prefer explicit parent-owned attributes over probing child objects withgetattr; use child checks only when the child owns the delegated behavior.
Preserve shared method signatures, argument order, return shapes, side effects, and error behavior unless every affected caller and interface is intentionally updated.
Do not add unused compatibility parameters, flags, attributes, constructor options, or model-specific options to shared helpers; keep one-off behavior at the integration boundary.
Normalize third-party return conventions at integration boundaries so core code receives the expected type and shape; avoid undocumented caller-side unwrapping.
Do not addtorch.no_grad,torch.inference_mode, or inference-mode wrappers. Do not add model freeze/unfreeze toggles; only disable globally enabled inference mode when a training path requires gradients.
Remove inference-only training behavior such as dropout while preserving checkpoint and state-dict compatibility; usenn.Identitywhen deleting a module would alter keys or ordering.
Keep imports at module scope except established optional-backend probes or imports required to avoid cycles; avoid unnecessarytry/exceptblocks and use specific exceptions with useful fallbacks.
Do not add workarounds for unsupported library versions, especially PyTorch exception-and-float-cast retries, unless a comment names the exact versions still requiring them.
Let unsupported model formats, invalid quantization metadata, and bad states fail with clear errors instead of silently degrading output.
Match local style, keep comments sparse and useful, and remove comments that merely restate obvious code.
Treat dtype, device placement, VRAM use, and offloading as correctness concerns across CPU, CUDA, ROCm, MPS, DirectML, XPU, NPU, and low-VRAM environments.
Prefer existing ComfyUI and Comfy Kitchen operations, quantization helpers, cast/offload helpe...
Files:
comfy_extras/nodes_compositor.py
**/*.{py,json}
📄 CodeRabbit inference engine (AGENTS.md)
Treat legacy combo,
io.Combo, andio.DynamicCombovalues affecting filesystem access as untrusted; revalidate them at load/save boundaries withfolder_paths, containment checks, or fixed allowlists.
Files:
comfy_extras/nodes_compositor.py
**/*.{py,md,txt,json}
📄 CodeRabbit inference engine (AGENTS.md)
Keep warning and info messages short and actionable, remove noisy or misleading logging, and make documentation edits concise, factual, and tied to changed behavior.
Files:
comfy_extras/nodes_compositor.py
**
⚙️ CodeRabbit configuration file
**: IMPORTANT: Only comment on issues directly introduced by this PR's code changes.
Treat AGENTS.md as mandatory repository policy, not optional style guidance.
Flag PR changes that violate AGENTS.md even when the code is otherwise functional.
In particular, enforce architecture boundaries, dtype/device/memory rules,
interface contracts, import style, no unnecessary try/except blocks, no inline
imports, no outbound internet paths in core ComfyUI, and narrow scoped fixes.
Prefer direct findings over suggestions when a rule is violated. Only ignore
AGENTS.md when it clearly conflicts with a newer explicit maintainer instruction
in the PR.
Do NOT flag pre-existing issues in code that was merely moved, re-indented,
de-indented, or reformatted without logic changes. If code appears in the diff
only due to whitespace or structural reformatting (e.g., removing awith:block),
treat it as unchanged. Contributors should not feel obligated to address
pre-existing issues outside the scope of their contribution.
Files:
comfy_extras/nodes_compositor.py
comfy_extras/**
⚙️ CodeRabbit configuration file
comfy_extras/**: Community-contributed extra nodes. Focus on:
- Consistency with node patterns (INPUT_TYPES, RETURN_TYPES, FUNCTION, CATEGORY)
- No breaking changes to existing node interfaces
Files:
comfy_extras/nodes_compositor.py
🧠 Learnings (6)
📚 Learning: 2026-02-21T14:01:41.482Z
Learnt from: pythongosssss
Repo: Comfy-Org/ComfyUI PR: 12555
File: comfy_extras/nodes_glsl.py:719-724
Timestamp: 2026-02-21T14:01:41.482Z
Learning: In PyOpenGL, bare Python scalars can be accepted for 1-element array parameters by NumberHandler. This means you can pass an int/float directly to OpenGL texture deletion (e.g., glDeleteTextures(tex)) without wrapping in a list. Verify function-specific expectations and ensure types match what the OpenGL call expects; use explicit lists only when the API requires an array.
Applied to files:
comfy_extras/nodes_compositor.py
📚 Learning: 2026-03-04T14:05:31.426Z
Learnt from: jtydhr88
Repo: Comfy-Org/ComfyUI PR: 12757
File: comfy_extras/nodes_custom_sampler.py:1069-1089
Timestamp: 2026-03-04T14:05:31.426Z
Learning: In the ComfyUI sampling pipeline, treat percent_to_sigma(0.0) as a sentinel value (999999999.9) that means starting from pure noise. This is consistent with BasicScheduler via calculate_sigmas. The SamplingPercentToSigma node’s return_actual_sigma flag differentiates this sentinel from sigma_max. Reviewers should not flag CurveToSigmas or similar nodes that rely on percent_to_sigma as bugs; downstream samplers are expected to handle the sentinel correctly. When reviewing related sampling-related code, assume this sentinel semantics unless there is explicit handling for a real sigma_max.
Applied to files:
comfy_extras/nodes_compositor.py
📚 Learning: 2026-04-04T13:29:15.653Z
Learnt from: kijai
Repo: Comfy-Org/ComfyUI PR: 13258
File: comfy_extras/nodes_frame_interpolation.py:151-189
Timestamp: 2026-04-04T13:29:15.653Z
Learning: In this ComfyUI codebase, node `execute()` inference is already run under a global `torch.inference_mode()` context established in the execution engine (e.g., `execution.py` around line ~732). During review, avoid recommending changes that wrap node inference loops in `torch.inference_mode()`—it is already applied, so such suggestions are likely redundant.
Applied to files:
comfy_extras/nodes_compositor.py
📚 Learning: 2026-05-09T18:40:40.199Z
Learnt from: kijai
Repo: Comfy-Org/ComfyUI PR: 13813
File: comfy_extras/nodes_wandancer.py:868-872
Timestamp: 2026-05-09T18:40:40.199Z
Learning: When building video/temporal decoding nodes that call ComfyUI’s VAE.decode (comfy/sd.py), leverage VAE.decode’s existing VRAM-aware chunking along dim 0. Reshape or transpose the latent so the temporal dimension T is folded into dim 0 (e.g., transform a latent of shape [B, T, C, H, W] into [B*T, C, H, W] before calling vae.decode). This lets VAE.decode do chunked decoding without needing an explicit per-frame loop inside the node itself.
Applied to files:
comfy_extras/nodes_compositor.py
📚 Learning: 2026-05-20T00:10:14.673Z
Learnt from: Pauan
Repo: Comfy-Org/ComfyUI PR: 13997
File: comfy_extras/nodes_string.py:12-25
Timestamp: 2026-05-20T00:10:14.673Z
Learning: In the ComfyUI `comfy_extras/` codebase, some nodes intentionally ship with a default input string that references parameters that may not yet be connected. If the default would raise a `KeyError` (e.g., examples like `MathExpression` default `a + b`, or `StringFormat` default `{a}` with `min=0` and autogrow inputs), treat it as an intentional “hint default” UX pattern, not a bug. During review, do not flag this behavior or recommend changing `min` to `1` or altering the default to an empty string solely to avoid the `KeyError`.
Applied to files:
comfy_extras/nodes_compositor.py
📚 Learning: 2026-07-26T18:37:44.213Z
Learnt from: jtydhr88
Repo: Comfy-Org/ComfyUI PR: 15090
File: comfy_extras/nodes_video.py:230-235
Timestamp: 2026-07-26T18:37:44.213Z
Learning: In ComfyUI node implementations under `comfy_extras`, do not add regular widget/prompt inputs to `fingerprint_inputs` if they are already included in the node cache signature via `comfy_execution/caching.py:get_immediate_node_signature` (it records every non-link prompt input as `(key, inputs[key])`). Reserve `fingerprint_inputs` only for out-of-band state that can change without changing the prompt inputs (e.g., the selected source file’s modification time). For example, inputs like `LoadVideo.edit` should not be redundantly added to `fingerprint_inputs`; use it only for things not represented in prompt inputs.
Applied to files:
comfy_extras/nodes_compositor.py
🔇 Additional comments (2)
comfy_extras/nodes_compositor.py (2)
23-24: LGTM!Also applies to: 176-176
407-410: LGTM!Also applies to: 415-425, 431-439
| bx = math.floor(min(xs)) | ||
| by = math.floor(min(ys)) | ||
| bw = max(1, math.ceil(max(xs)) - bx) | ||
| bh = max(1, math.ceil(max(ys)) - by) |
There was a problem hiding this comment.
x and y arrive as floats (_layer_params reads them with _number, and the editor writes float transforms), but the layer is snapped to the integer bounds rather than resampled onto them, so the sub-pixel part of the position is discarded. The browser preview does not discard it: place.ts does ctx.translate(transform.x + transform.w / 2, ...) and lets canvas2d resample, so the same state renders up to ~1px apart in the two places.
Measured against this branch:
x= 10.0 bx=10 bw=100 ox=0 -> drawn at 10 (browser draws at 10.00) error +0.00px
x= 10.25 bx=10 bw=101 ox=0 -> drawn at 10 (browser draws at 10.25) error -0.25px
x= 10.5 bx=10 bw=101 ox=0 -> drawn at 10 (browser draws at 10.50) error -0.50px
x= 10.75 bx=10 bw=101 ox=0 -> drawn at 10 (browser draws at 10.75) error -0.75px
x= 11.0 bx=11 bw=100 ox=0 -> drawn at 11 (browser draws at 11.00) error +0.00px
(ox is _place_in_bounds's (bw - aw) // 2, which can contribute another half pixel of its own once rotation widens the bounds.)
The user-visible shape of this is worse than "up to 1px off": because it snaps rather than resamples, nudging a layer by half a pixel in the editor changes the preview and changes nothing in the render until it crosses a pixel boundary. For a node whose entire contract is "the render matches what you arranged", that reads as the compositor ignoring an edit.
Not proposing a patch inline because the fix is a small rework rather than a line change: the cleanest version is to stop producing an integer-aligned intermediate at all and let one resample carry scale, rotation and the fractional translate together — Image.transform(canvas_size, Image.Transform.AFFINE, matrix, resample=BICUBIC) with the inverse of the layer's transform, which also removes _place_in_bounds's centring. Happy to do it as a follow-up PR if you want it; flagging it now because it is the kind of thing that gets filed as "the compositor is off by one" later.
Golden-fixture PR for the blend side of this is #15373.
| rgb = linear_to_srgb(np.clip(canvas[..., :3], 0.0, 1.0)) | ||
| alpha = np.clip(canvas[..., 3:4], 0.0, 1.0) |
There was a problem hiding this comment.
This is the only clamp in the stack, so between layers the canvas can hold values outside [0,1] and the next layer blends against them. linear-dodge (i + l), linear-light (i + 2l - 1), linear-burn, grain-merge, grain-extract and the HSL modes all leave that range routinely.
I want to be precise about what this is and is not, because the obvious reading is wrong:
- It is not a preview/render divergence. I checked, expecting one —
webglCompositor.ts:276-281allocates the layer target asRGBA16F/HALF_FLOAT, so the GPU carries over-range values too and also clamps once at the end. The two agree. - It is a divergence from core.
ImageBlendclamps its result per operation, and so does Photoshop. So a two-layer stack that goeslinear-dodgethenmultiplygives one answer in this node and a different one in core's nodes, for the same pixels.
Which of the two is right is a product call about 22 modes' output rather than a bug fix, which is why I have not touched it in #15373 — that PR only fixes the one place where the shader and this file genuinely disagree (luminosity) and pins all 26 modes to a shared golden fixture so the answer, whichever it is, stops drifting.
Concretely, if the answer is "match core", it is one line — clamp comp inside blend_composite before run_composite — plus the same clamp in blendPixel, plus regenerating compositor_blend_golden.json. If the answer is "keep the wide gamut between layers because that is what a real compositor does", then it is worth a sentence in the node description, because it is a real behavioural difference from every other blend node in core and someone will report it.
This is the "glsl, fe, be, this node, core nodes are all slightly different" item — with blend.ts deleted and luminosity fixed, core is the last one left.
Answering "is this a structural constraint of autogrow, or is there something we can do via some redesign"(@christian-byrne asked this about Short version: the Python side is not a structural constraint and is a 3-line fix. But the Python side is not where the mask incident came from, and the part that is structural already has a solution shipped in the frontend that nobody is using. 1. The slot index is not lost on the Python sideThe value handed to Running the real The keyed version is: def by_index(values: dict | None) -> dict[int, Any]:
return {int(k.rsplit("_", 1)[-1]): v for k, v in (values or {}).items() if v is not None}So: not structural. Worth knowing because the same dense-list pattern is still live in core, in 2. The part that is structural is in the frontend, and no server-side fix can reach it
So two parallel autogrow groups can never express a pairing, no matter what the Python does. That much is structural. 3. The redesign already exists in the frontend — one autogrow group whose template is the pair
The gap is on the Python side only: That is a direct answer to "I really wish we either had RGBA type in core or our autogrow could better express paired x + 2n growing" — the frontend half of "paired x + 2n growing" is already written. It needs a What this means for
|
…sity (#15373) Blending exists in two implementations - the numpy compositor and the layerBlend.frag shader that drives the live preview - with nothing holding them together. They have already diverged once (the safeDiv operand), and a divergence only shows up to the user as 'the render does not match the preview'. Adds compositor_blend_golden.json: every mode, at both endpoints, the midpoint and inside each epsilon guard. Any implementation of these 26 modes must reproduce it. compositor_blend_test.py pins the numpy side to it and additionally spells out the boundary rules by hand, so the guards cannot be re-broken by regenerating the fixture. Diffing the shader against the numpy implementation over that grid leaves exactly one mismatch: luminosity. safe_div guards the denominator and returns 0, so a luminosity layer over a black or near-black backdrop disappears. The backdrop has no hue or saturation to preserve there, so the result should be a neutral grey at the layer's luminance - which is also the analytic limit of i * lum(l)/lum(i) as the backdrop approaches black. The matching four-line shader change is proposed on the frontend PR; with both applied all 26 modes agree. Also clamps layer opacity to [0, 1]. The layer state round-trips through the saved workflow and is accepted verbatim on /prompt, so it is untrusted input; the canvas is only clamped once, after the last layer, so an out-of-range coverage multiplier changes the blend of every layer above it. _parse_background already clamps the same field.
christian-byrne
left a comment
christian-byrne
left a comment
There was a problem hiding this comment.
Full review of the whole PR at 3b3c61dd, from a fresh worktree. This is our first proper pass over it, so I ran it as six parallel lenses (compositing math, tensor/dtype contracts, the LAYERS and COMPOSITOR wire formats, error paths, memory/perf, schema conventions and tests) and deduplicated before posting. Everything below was reproduced against this head, not inferred.
First, the parts that are just good
You cut ~8,400 lines off this thing and it is a much better PR for it. Specifically, and all verified rather than taken on trust:
stack_imagesis gone, and with it the last-writer-wins path that silently discarded N-1 layers.sort_autogrow_images,expand_batch_frames,layout_bboxesand the whole autogrow slot machinery are gone too, which removes the dense-list slot-shifting bug class entirely rather than patching it.- Alpha is correct end to end now.
frame_alpha:88reads channel 3,_prepare_layer_bitmap:318rebuilds a true RGBA bitmap, compositing happens in linear light, and the node emits RGBA plus a MASK. The[..., :3]slice at:310still greps as if it were lossy and no longer is. - The
LAYERStype landed and matches what was agreed: requiredz_indexwith a stable sort,x/yorigin, per-layeropacity/blend_mode/visible/flip_h/flip_v, and aDocumentwrapper carryingversionandcanvas. I checked the stable-sort tie behaviour against the tooltip at:554and it is accurate. - The version gate is real on both sides.
parse_layer_state:248-250rejects a foreign version, and the frontend genuinely writes it (compositorLayerState.ts,LAYER_STATE_VERSION = 1, written byextractLayerState, validated on read). An unversioned persisted blob was the single biggest thing we were worried about and it is closed. - The transparent default landed, with tests, and the replay path agrees with it (the FE's
DEFAULT_BACKGROUND_ENTRYis alsovisible: false). Replied on that thread separately. - Blend parity with the shader is now exact. I transcribed
layerBlend.fraginto an independent oracle and diffed the full pipeline (blend, space round trip, composite operator) over 412,776 cases: 26 modes x their blend space and composite x 7 RGB triples including over-range and negative x 9 backdrop alphas x 9 layer alphas x 4 opacities, with epsilon-scale boundaries. Worst relative difference1.31e-06, zero semantic mismatches.9e5cb779really did fix thesafe_divinversion andd36bc61areally did fix luminosity. TheLUM_R/G/Bconstants are byte-identical tolayerBlend.frag:33. I am not raising anything on blend math. - Things I went looking for and could not find: no NaN or Inf anywhere (I stacked 50 layers of the worst-growing modes, canvas reaches +8723 and stays finite), sRGB round trip lossless to 0 error across all 256 8-bit codes,
_composite_union's algebra is textbook straight-alpha source-over-with-blend with nothing wrongly premultiplied, no dead code left inresolve_mode,Compositor.Input's 12 positionals line up correctly againstWidgetInput.__init__, and the "PIL resampling is not alpha-aware" claim is false at Pillow 12 (it premultiplies internally). Also, for the record,Compositor.Inputas a socketless widget type is the house pattern (Colors.Input,BoundingBoxes.Input), not a deviation, so no rewrite needed there.
Two things I would fix before this merges
Both are inline. Both are reachable without hand-editing anything, and both are small fixes.
- A single
Add Layerwith a negativexcrashes the prompt.canvas_extentonly takesmax(x + w)and never the minimum, sox=-100on a 64px image gives a canvas width of-36andnp.zerosraisesValueError: negative dimensions are not allowed.x=-64gives a legally-zero-width canvas that survives the composite and then dies insideUI.PreviewImage.AddLayerdeclaresmin=-MAX_RESOLUTIONonx/y, so this is default widget range, not abuse. - Layer
transform.w/hare unbounded from the untrusted blob. You wrote the threat model yourself nine lines above, foropacity.w/hdid not get the same treatment and they gate two allocations.
Should-fix, inline
Six more inline, roughly in order of how likely a user is to hit them. The one I would look at first is state_from_items:170: 25 of the 26 blend modes Add Layer offers are silent no-ops on the bottom layer, and the node gives no diagnostic.
Follow-ups, not blocking
Grouping these rather than posting fifteen more inline comments. Happy to file any of them as issues or send patches.
Correctness / robustness
_parse_order:224requires a complete permutation, and rejects a subset by returningNone, after whichcomposite_from_state:377renders every index and_layer_paramsdefaults the missing entries. So a layer the editor deliberately excluded gets composited at natural size at the origin at full opacity instead of being dropped. Worth accepting a subset-permutation and skippingNoneentries. Separately, a corruptorderis dropped inside a successful replay, so the user's positions and opacities are restored but their stacking silently reverts, with no stale flag._item_mask_frame:51-58: an image batch of 4 with a mask batch of 2 gives frames 2 and 3Noneand they render fully opaque. Two masked layers and two solid rectangles, no warning.nodes_compositor.py:249: the version gate uses!=, so1.0and JSONtrueboth pass. Andversion is not Nonemakes a missing version indistinguishable from1, which is the exact ambiguity the gate exists to remove. Since the FE branch is unmerged there is no legacy corpus, so you can requireversion == 1today and never be able to again._number:276acceptsnan/inf(isinstance(nan, float)is True), which then raises out ofround()/math.floor().json.loadsacceptsNaNby default.math.isfinitecloses it.- Neither new module logs anything. 21 of 131
comfy_extrasmodules uselogging, and onelogging.warningat theparse_layer_staterejection sites would make all of the above diagnosable without changing behaviour. frame_alpha:94:interpolate(..., mode="bilinear")withoutantialias=Truealiases when downscaling a mask larger than the image.
Perf, all measured, all deferrable
_fill_background:350runs a full union composite to paint a solid colour: 58.4 ms vs 3.1 ms for a direct fill at 1024², bit-identical output. Four lines.srgb_to_linearevaluates the**2.4branch over the whole array then discards most of it (22.5 ms of 29.6 ms at 1024²). After the PIL round trip the domain is exactly 256 values, so a 256-entry LUT is bit-exact, not an approximation (verified: max error 0.0 over all 256).- A
normal/no-transform fast path measured 11.5 ms vs 101.4 ms per layer at 1024², so 50 layers goes 5.07 s to 0.58 s. Well covered by the golden fixture, so low risk. - On CodeRabbit's fingerprint thread: I would not take the strided subsample. That fingerprint is the correctness gate for replaying a saved composition, and subsampling means an edit confined to unsampled pixels silently replays a stale composition. But the cost is not sha256, it is the
rint/clip/astypechain at:124. Hashing the raw float32 bytes measured 30.2 ms vs 81.6 ms per layer at 2048² despite hashing 4x more data, and it is stricter. Also, that same quantisation is computed twice per layer, at:124and again at:311. - Peak transient is ~8x canvas bytes (4.3x by
tracemalloc, 7.7-9.7x by RSS). AtMAX_RESOLUTIONthat is 4.30 GB base and ~34 GB transient. There is no total-pixel cap, only per-axis. The canvas guard ordering is correct though, it raises before allocating. - Every layer is force-quantised to 8 bit before compositing (
:311), max error 1/510. The FE composites intoRGBA16F, so stacked low-contrast layers will band on the server and not in the preview.
Schema and conventions
category="image"on both nodes;image/compositingalready exists and is exactly this neighbourhood (ImageCompositeMasked,PorterDuffImageComposite,SplitImageWithAlpha,JoinImageWithAlpha). Two string literals.- No
description=and nosearch_aliases=on either schema (I checked the generated schema: both are empty). The user-visible names are "Create Layered Image" and "Add Layer", so the words "composite" and "compositor" appear nowhere a search would match, and a user typing "composite" getsImageCompositeMaskedand never sees this. Both of its nearest neighbours set aliases (nodes_mask.py:82,nodes_compositing.py:112) andCreateBoundingBoxessets a description. Two lines. visibleis honoured all the way through (:74to:153to:289to:384) butAddLayerhas novisibleinput, so API, headless and agent callers can never hide a layer.flip_h/flip_vgot inputs and are strictly less useful. One input plus two lines inexecute.compositoris a required input (confirmed:optional=Falseon the generated schema), so aPOST /promptthat omits it fails validation withrequired_input_missing(execution.py:901-906).executealready defaults it toNone, sooptional=Truematches what the code expects and removes the need for programmatic callers to hand-write a widget blob.from comfy_extras.compositor_blend import _LAYER_MODESimports a private name across modules and then uses it as the publicblend_modeoption list. Rename toLAYER_MODES, two call sites.MAX_LAYERS = 50raises at:78-81but appears in no tooltip.- Outputs have no
display_name, so/object_infonames themIMAGE/MASK. - The tooltips themselves I checked clause by clause against the code and they are accurate, including the mask polarity at
:508againstframe_alpha:97-98. Nice.
The PR description is now stale. It still describes autogrow image_0..49, a bboxes input, compositing over a white background, and a fallback to a plain stack. None of that exists at head. Worth a rewrite before merge since it becomes the squash commit message.
Tests
Being fair about the baseline first: comfy_extras/ has 131 modules and tests-unit/comfy_extras_test/ covers four of them on the merge base. This PR ships 76 passing tests, a 2,242-line golden fixture, a documented regeneration script, and hand-written boundary rules that are deliberately fixture-independent so regenerating the golden cannot launder a behaviour change. That is comfortably the best-tested contribution in that directory and I am not going to ask for "more tests" as a general matter. I ran both files: 67 passed and 9 passed.
Two specific things though.
The golden fixture is generated from the Python it tests. compositor_blend_fixture_gen.py:27 imports blend_pixel from compositor_blend and :60 calls it, and I confirmed re-running the generator reproduces the committed JSON byte for byte. It is an excellent regression pin, and the docstring's "shared contract that layerBlend.frag must reproduce" overstates what it can do: nothing on the FE branch reads it (there is no blend test there, only the .frag), so it structurally cannot catch a wrong port from the shader. That is the one failure mode the docstring claims it guards. Softening those four lines is cheap; the real cross-check needs a headless-GL harness in the frontend repo and is a genuine follow-up, not this PR. (Full disclosure, that fixture came in via #15373, which is mine, so this is me marking my own homework.) The parity oracle I ran for this review is the thing that actually establishes agreement and it lives nowhere in the repo, so I will offer it as a PR.
Highest value-per-line test asks, all cheap, all additions to the existing files:
placed_boundsagainstImage.rotate(expand=True).sizeover a few angles. This is how the rotation bug below was found; nothing today would catch it.- One
AddLayerchain intoImageCompositor.execute. The two nodes ship together and nothing tests the contract between them;compositor_node_test.pyimports neither class. - A
parse_layer_staterejection table. There are seven silentreturn Nonepaths and the new version gate, and the file's own docstring says this input is untrusted. composite_outputs' channel-count branch.test_uncovered_canvas_stays_transparentasserts a 4-channel result but callscomposite_from_statedirectly, so it never reachescomposite_outputs.input_fingerprintsstability and sensitivity. The head commit changed what goes into that hash, andframe["opacity"]goes throughrepr()unnormalised, so1and1.0fingerprint differently.AddLayerhappens to coerce withfloat(opacity), but nothing enforces that for a third-partyLAYERSproducer.
One CI caveat worth stating explicitly, not a criticism of this PR. .github/workflows/test-unit.yml sets continue-on-error: true at job level:
jobs:
test:
strategy:
matrix:
os: [ubuntu-latest, windows-2022, macos-latest]
runs-on: ${{ matrix.os }}
continue-on-error: true # line 15So a unit-test failure reports the job as success and cannot block a PR. Your tests genuinely do pass at this head (I checked the step conclusions on all three OSes, not just the job conclusion), but "CI is green" is not evidence that they will keep passing. tests-unit/comfy_extras_test/ is collected (pytest.ini testpaths, plus the workflow passes tests-unit explicitly), so the tests do run.
One thing I am not asking you to change
COMPOSITOR and LAYERS coexisting looked wrong to me until I traced it. LAYERS is the data on the wire and COMPOSITOR is the editor's saved arrangement of that data, and the precedence rule is coherent: :465 replays the saved arrangement only when its fingerprints match the current frames, otherwise the LAYERS document wins and the FE is told to reset. That is the right split and I would leave it. The thing worth tightening is not the split, it is what happens on rejection, which is the :475 comment below.
Thanks for turning this around as fast as you have, and for doing the co-location the moment it was asked for. None of the above is a rewrite.
|
|
||
| def canvas_extent(frames: list[dict]) -> tuple[int, int]: | ||
| return ( | ||
| max(frame["x"] + frame["tensor"].shape[2] for frame in frames), |
There was a problem hiding this comment.
issue (blocking): a single Add Layer with a negative x crashes the prompt.
canvas_extent only takes max(x + w) and never the minimum, so a layer placed at a negative offset shrinks the canvas instead of widening it. AddLayer declares min=-MAX_RESOLUTION on x/y (:520, :528), so this is reachable with default widget range and one node, no hand-edited state.
Reproduced against this head, one Add Layer with a 64x64 image and nothing else in the graph:
x= -63 canvas_extent=(1, 64) -> out (1, 64, 1, 4) # 1px-wide output, no error
x= -64 canvas_extent=(0, 64) -> out (1, 64, 0, 4) # legal, then dies in UI.PreviewImage
x= -100 canvas_extent=(-36, 64) -> ValueError: negative dimensions are not allowed
x=-1000 canvas_extent=(-936, 64) -> ValueError: negative dimensions are not allowed
The x=-100 traceback lands on np.zeros((ch, cw, 4)) at :372, which gives the user a bare numpy error with nothing actionable in it. The x=-64 case is worse because the composite succeeds and it blows up later inside PIL with SystemError: tile cannot extend outside image.
Two separable things here, and I would do both:
- Extend the guard at
:367to rejectcw <= 0 or ch <= 0with the same style of message as theMAX_RESOLUTIONbranch, so the failure is at least legible. - Even when it does not crash, a layer at negative
xis silently cropped at:393(x0 = max(bx, 0)) and never widens the canvas. TwoAdd Layernodes, one atx=-5000, and the output simply does not contain that layer, with no warning. Either grow the canvas tomax(x+w) - min(x, 0)and shift the origin, or at minimumlogging.warningthat layer N falls outside the derived canvas.
Clamping only (1) turns the crash into silent data loss, which is why I would not stop there.
| "blend": blend if isinstance(blend, str) else "normal", | ||
| "x": _number(transform, "x", 0.0), | ||
| "y": _number(transform, "y", 0.0), | ||
| "w": _number(transform, "w", natural_w), |
There was a problem hiding this comment.
issue (blocking): layer transform.w/h are unbounded, and they gate two allocations.
You already wrote the threat model for this, nine lines up at :290-294: the layer state round-trips through the saved workflow and can be posted directly to /prompt. opacity got the clamp and the comment. w and h did not, and unlike opacity the consequence is an allocation rather than a wrong colour.
The MAX_RESOLUTION guard at :367 bounds the canvas. Nothing bounds the layer:
_layer_params({'transform': {'w': 40000, 'h': 40000}}, 64, 64)
-> w=40000.0 h=40000.0 (passed straight through)
which reaches _prepare_layer_bitmap:323 as a 40000x40000 LANCZOS resize target and _place_in_bounds:340 as np.zeros((40000, 40000, 4), float32) = 25.6 GB, on a canvas that may legitimately be 64x64.
Two reasons I think this is worth fixing now rather than later:
- It is reachable through ordinary editor use, not just crafted JSON. Scaling a layer larger than the canvas is a normal compositing action; on a 4096 canvas a 3x-scaled layer is a 2.4 GB
bufplus a 604 MB PIL destination, all of it materialised and then immediately clipped to the canvas at:397. - The fingerprint is published back to the client at
:481(ui_dict["compositor_inputs"] = fp), so the replay gate at:465is trivially satisfiable: run once, read the fingerprint, resubmit with a largew.
Deadline-safe fix is three or four lines in the same idiom as the opacity clamp right above: bound w/h to [1, MAX_RESOLUTION] and reject non-finite values. json.loads accepts Infinity and NaN by default, and _number lets them through since isinstance(nan, float) is True, after which round(inf) raises OverflowError and math.floor(nan) raises ValueError, both uncaught.
The properly correct version (intersect with the canvas before allocating, so an off-canvas layer costs its visible area rather than its nominal area) is a real rework of _prepare_layer_bitmap plus _place_in_bounds, and I would not ask for that before the deadline.
| "canvas": canvas, | ||
| "layers": layers, | ||
| "inputs": None, | ||
| "background": {"color": "#ffffff", "opacity": 1.0, "visible": False}, |
There was a problem hiding this comment.
issue: 25 of the 26 blend modes Add Layer offers are silent no-ops on the bottom layer.
state_from_items hardcodes an invisible background, so the graph-only path starts on a fully transparent canvas. Every mode except normal resolves to clip-to-backdrop (compositor_blend.py:258-285), which returns the backdrop unchanged when in_a == 0. Reproduced end to end through AddLayer.execute at this head, single layer, nothing else in the graph:
normal -> rgba=[0.7843, 0.3922, 0.1961, 1.0]
multiply -> rgba=[0.0, 0.0, 0.0, 0.0]
screen -> rgba=[0.0, 0.0, 0.0, 0.0]
overlay -> rgba=[0.0, 0.0, 0.0, 0.0]
difference -> rgba=[0.0, 0.0, 0.0, 0.0]
luminosity -> rgba=[0.0, 0.0, 0.0, 0.0]
I want to be precise that this is not a bug in the algebra. It is correct Photoshop semantics and it matches the shader exactly, which I verified separately. The problem is purely that AddLayer:541-547 offers all 26 modes in a combo with no way to turn a background on, so a user who picks multiply on a single-layer stack gets a fully transparent black image and no diagnostic anywhere.
Three ways out, in increasing order of effort: say it in the blend_mode tooltip; expose a background input on the node; or default visible to True here. I would lean against the last one because it undoes the transparent default you just landed, so probably tooltip now and a background input as a follow-up. Flagging it because "I set the blend mode and the image went blank" is the kind of thing that gets filed as a broken node.
| if out.shape[-1] != 4: | ||
| return out, torch.zeros(out.shape[:3], dtype=torch.float32) | ||
| alpha = out[..., 3] | ||
| if bool((alpha >= 1.0 - OPAQUE_EPSILON).all()): |
There was a problem hiding this comment.
issue: the IMAGE output's channel count depends on pixel content, so the same graph emits 3 channels on one run and 4 on the next.
A single pixel at alpha 0.9985 flips this branch. The moment a user hides a layer, or a layer stops covering the full canvas, downstream nodes get a different arity than they did on the previous run, with no validation error because the graph is already built.
4-channel IMAGE is fine in itself and has precedent (JoinImageWithAlpha, nodes_compositing.py:209), but it is deterministic there, so a user finds out on the first run. Here the failure is deferred and lands somewhere else. Concrete consumers, all reachable straight off an IMAGE socket:
ImageInvert(nodes.py:1933) doess = 1.0 - imagewith no slice, so it inverts alpha too: an opaque composite comes out fully transparent. No error, just a wrong image.ControlNetApply/ControlNetApplyAdvanced(nodes.py:903,:941) doimage.movedim(-1, 1)with no slice. It survivescommon_upscaleand dies at the control model'sConv2d(3, ...), i.e. a channel-mismatchRuntimeErrorreported inside KSampler, nowhere near the compositor. VAE-based controlnets are fine (comfy/sd.py:1051-1052slices), so it fails inconsistently depending on controlnet type.ImageUpscaleWithModel(nodes_upscale_model.py:77) raises.ImageQuantize(nodes_post_processing.py:163) reinterprets the RGBA buffer as RGB triplets (PIL does not raise on an explicitmode='RGB') then raises on the assignment.ImageBlend(nodes_post_processing.py:40-65):image_alpha_fixmakes the RGBA operand win, anddifferencethen computesalpha - 1.0which clamps to 0, so the whole image goes transparent.
Verified not broken: SaveImage, PreviewImage, UI.PreviewImage, SaveAnimatedPNG/WEBP, VAEEncode, ImageBatch, ImageStitch, ImageCompositeMasked, ImageScale.
The repo's established contract for transparency is RGB IMAGE plus a separate MASK: LoadImage never emits 4 channels (nodes.py:1745-1746, and the webp fallback at :1758 explicitly does convert("RGB")), and SplitImageWithAlpha exists precisely to convert RGBA back into that shape. This node already has the MASK output, and its polarity already matches (:417 1.0 - alpha versus nodes_compositing.py:186 1.0 - alpha), so IMAGE(rgb) -> JoinImageWithAlpha <- MASK round-trips exactly.
Suggestion: always return out[..., :3] plus the mask, and drop OPAQUE_EPSILON. About three lines, and it makes the output arity a static property of the node instead of a property of the pixels. If you would rather keep 4-channel output, that is defensible, but then I would make it unconditional rather than content-dependent, since the deferred-and-inconsistent part is what makes it hard to debug.
| corners = ((-hw, -hh), (hw, -hh), (hw, hh), (-hw, hh)) | ||
| xs = [cx + dx * cos - dy * sin for dx, dy in corners] | ||
| ys = [cy + dx * sin + dy * cos for dx, dy in corners] | ||
| bx = math.floor(min(xs)) |
There was a problem hiding this comment.
issue: float dust in the rotation matrix shifts the origin by a whole pixel at cardinal angles.
Separate from the sub-pixel translation thread on :330. This one is exact-angle, and it is a two-line fix rather than a rework.
math.sin(math.pi) is 1.2246e-16, so min(ys) comes out at -6.1e-15 and math.floor turns that into a full pixel of origin shift. Measured against what PIL actually produces, over (w,h) in {(64,64), (100,50), (33,17)} x 10 angles:
w,h deg | placed_bounds vs PIL rotate(expand) origin
64, 64 90 | (65,65) vs (64,64) (-1,-1)
64, 64 180 | (65,65) vs (64,64) (-1,-1)
64, 64 270 | (65,65) vs (64,64) (-1,-1)
100, 50 180 | (101,52) vs (100,50) (-1,-1)
100, 50 270 | (51,101) vs (50,100) (24,-26)
33, 17 180 | (34,18) vs (33,17) (-1,-1)
33, 17 270 | (19,34) vs (17,33) (7,-9)
MISMATCHES: 7 of 30
placed_bounds(0, 0, 100, 50, math.pi) = (-1, -1, 101, 52) expected (0, 0, 100, 50)
The box is one pixel larger than the bitmap rather than smaller, so nothing gets cropped, but _place_in_bounds then centres the PIL image inside the oversized box with ox = (bw - aw) // 2 and composite_from_state:397 slices using the drifted bx/by. Net effect: a layer rotated exactly 90, 180 or 270 degrees lands a pixel off from where the editor put it, which is exactly the "render does not match the preview" class the test docstring says the suite exists to prevent.
Only reachable with a non-zero rotation, and rotation can only come from saved editor state, so it is not an every-render defect. But it is cheap: snap near-integers before floor/ceil, or derive the box from the PIL image size directly so the two cannot disagree by construction. The single-AFFINE-transform rework proposed on the :330 thread would subsume this, so if you go that way this comes out for free.
Related, and it survives either fix: _place_in_bounds:341's ox = (bw - aw) // 2 is a no-op whenever w is an integer (bw - aw is 0 or 1, so ox is always 0), which makes the placement a pure leftward bias rather than nearest-rounding. For fractional w the same expression accidentally becomes a correct rounder. So x=0.75, w=10 lands at 0 and x=0.75, w=10.5 lands at 1: a half-pixel change in width moves the layer a full pixel. Measured mean error over w in {10, 10.5, 11, 33.3, 63.7} is -0.339 px where an unbiased rounder would be ~0.00.
| alphas = [frame_alpha(frame["tensor"], frame["mask"]) for frame in frames] | ||
|
|
||
| layer_refs = [] | ||
| for tensor, alpha in zip(tensors, alphas): |
There was a problem hiding this comment.
suggestion: this writes a full-resolution PNG per layer per frame on every execution, unconditionally.
UI.PreviewImage.__init__ encodes and writes eagerly (_ui.py:389-396 into save_images into img.save(..., compress_level=1)), so this loop is N encodes on the execution thread before any replay decision has been made, plus one more for the output at :478.
Measured on this box (loaded, so treat as an upper bound, and random noise is worst-case for PNG):
1024x1024 RGBA compress_level=1: 117 ms, 4.2 MB -> 51 previews = 6.0 s
2048x2048 RGBA compress_level=1: 464 ms, 16.8 MB -> 51 previews = 23.6 s
Two things make it worse than a one-off cost:
- It is not skipped when nothing changed.
_send_cached_ui(execution.py:430-448) replays cached UI only on a true cache hit, in which caseexecutenever runs at all. Butcompositoris an input, so every editor tweak (drag a layer, change a blend mode) changes the widget, misses the cache, re-runs the node, and re-encodes all N layer previews even though the layer pixels are byte-identical. That cost is paid on every iteration of the edit loop, not once.has_intermediate_output=Truedoes not help here because it only governs the cached-replay path you never reach. cleanup_temp()runs at startup and shutdown only (main.py:510,:593), so at 2048x2048 that is ~549 MB of temp PNGs per iteration accumulating for the whole session.
They are also written before the canvas guard at :367, so a workflow that is about to be rejected for an oversized canvas pays all N encodes first.
Cheapest fix is to bound the layer previews to a thumbnail (something like max(w, h) <= 1024). The editor is drawing these into canvas elements, and full resolution is also what is being pushed over HTTP, so I would expect it to be invisible in the UI for a large win. Moving the preview generation behind the early-out for the canvas guard is nearly free and worth doing regardless.
| @comfytype(io_type="COMPOSITOR") | ||
| class Compositor(ComfyTypeIO): | ||
| class LayerState(TypedDict): | ||
| canvas: dict |
There was a problem hiding this comment.
suggestion: LayerState omits version, which is the field that gates the whole thing.
parse_layer_state:248-250 reads state.get("version") and rejects anything that is not 1, and the frontend genuinely writes it (compositorLayerState.ts: LAYER_STATE_VERSION = 1, emitted by extractLayerState, validated on read). So the runtime contract is versioned on both sides, which is great and closes the thing I was most worried about on this PR.
The published TypedDict is the part that does not say so. Once this lands in __all__ it is the permanent description of the widget payload for anyone writing a custom node or a client, and it currently documents canvas, background, inputs, order and layers but not the one field that decides whether the payload is accepted at all. One line: version: NotRequired[int].
Two adjacent things while you are in here, both cheap:
canvas: dictis the wire shape ({w, h}), butparse_layer_state:268normalises it to a tuple andstate_from_items:167emits a tuple. So "aLayerState" means two structurally incompatible things depending on which function produced it, under one nominal type, and round-tripping astate_from_itemsoutput back throughparse_layer_stateis silently rejected. Worth either naming the internal one differently or havingstate_from_itemsemit the wire shape.ImageCompositor.executeis annotatedcompositor: io.Compositor.Type = None, butTypeis aTypedDictandNoneis not assignable;io.Compositor.Type | None = Noneis what is meant. Same atnodes_compositor.py:575forlayers.
| ) | ||
| else: | ||
| out = torch.zeros((1, 64, 64, 3), dtype=torch.float32) | ||
| state_stale = layer_state_provided(raw_state) and not replay |
There was a problem hiding this comment.
issue: this bit conflates "your inputs changed" with "your saved state was rejected", and the frontend's response to it is destructive.
state_stale is set whenever a state was provided and did not replay. That fires identically for the expected case (the user nudged a layer, so the fingerprints moved) and for the structural cases (parse_layer_state returned None). There are thirteen distinct ways to get that None, including the version gate at :248-250:
single x change on ONE layer -> compositor_state_stale: [True]
{"version": 2, ...} payload -> compositor_state_stale: [True] # identical signal
malformed JSON -> compositor_state_stale: [True]
canvas w=0 -> compositor_state_stale: [True]
The frontend then treats that bit as permission to discard (imageCompositor.ts:49-51):
if (output.compositor_state_stale?.[0]) {
resetCompositorStateWidgets(node) // writes {} into widget.value and widgets_values
node.graph?.setDirtyCanvas(true)
}So a user on a newer frontend running against an older backend has their entire saved composition erased, precisely because the version gate did its job. Version-gating to avoid mis-rendering is the right call; wiring the rejection into a destructive client-side reset gives back more than the gate wins. They will re-do the layout, re-save, and lose it again, and the only thing they are told is "your inputs changed".
The expression right here already has both pieces of information (state is None versus state["inputs"] != fp), so splitting it is small: keep compositor_state_stale for the fingerprint case and add something like compositor_state_rejected for the structural case, with the frontend preserving the widget value and surfacing a toast on the latter. Needs the paired FE change, so worth deciding now while both PRs are open.
Second, smaller thing on the same line: the comparison at :465 is whole-list equality over a per-layer fingerprint list, so one layer moving by a pixel discards the arrangement of all twelve. You already have per-layer granularity in fp; matching per index and resetting only the layers whose fingerprint moved would be a much better edit loop, and it composes with the split above. (Same all-or-nothing logic on the FE side in layerStateInputsMatch.)
Whichever way this goes, the behaviour deserves a sentence in a tooltip. :433 currently says "A saved composition that matches the current inputs takes priority" and :522 says "Initial horizontal placement on the canvas"; neither tells the user that changing x throws away their saved arrangement.
| visible: NotRequired[bool] | ||
| flip_h: NotRequired[bool] | ||
| flip_v: NotRequired[bool] | ||
| color: NotRequired[str] |
There was a problem hiding this comment.
suggestion: color has no producer and no consumer, and Document.version has no reader. Both are about to become permanent public API.
Traced every field of LayerItem at this head:
| field | producer | consumer |
|---|---|---|
image, x, y, mask, z_index, name, opacity, blend_mode, flip_h, flip_v |
AddLayer |
yes |
type: Literal["raster"] |
AddLayer:578 |
nothing reads it |
visible |
nothing sets it | :74 -> :289 -> :384 |
color |
nothing | nothing |
color only means anything on a fill layer, and document_items:32 requires isinstance(item.get("image"), torch.Tensor), so a fill item is silently dropped:
[{"type": "fill", "color": "#ff0000", "z_index": 0}, {"image": <tensor>, "z_index": 1}]
-> items kept: 1 of 2
The failure mode once this is in __all__ is a custom-node author reading the TypedDict, emitting a fill layer, and getting a no-op with no error. Adding a field to a public type later is free; removing one is not. I would drop color (and either check type in document_items or drop it too) until fill layers actually exist.
Same shape one line down at :868: Document.version is written by AddLayer:597 and read by nobody.
document_items({"version": 2, "layers": [{"image": <tensor>, "z_index": 0}]}) -> 1 item kept
That is the "looks like a migration hook and is not" case, and it is the one field a future consumer will trust. parse_layer_state:248-250 already has the two-line pattern for the sibling type; mirroring it in document_items costs nothing and means a v2 producer against a v1 consumer errors instead of silently mis-rendering.
Also worth a decision while the type is still private: blend_mode is NotRequired[str] rather than a Literal of the 26 modes, so a typo is accepted and silently resolves to normal via resolve_mode. From the graph it is constrained by the combo on AddLayer, but a custom node producing LAYERS has no such guard.
| display_name="Create Layered Image", | ||
| category="image", | ||
| is_experimental=True, | ||
| is_output_node=True, |
There was a problem hiding this comment.
suggestion (non-blocking): is_output_node=True together with has_intermediate_output=True looks like it may be one flag too many.
The docstring for the flag (comfy_api/latest/_io.py:1689-1697) describes them as alternatives: an intermediate-output node "behave[s] like output nodes ... but do[es] NOT automatically get added to the execution list ... Use this for nodes with interactive/operable UI regions." Setting is_output_node=True puts it back on the list, so Create Layered Image executes on every queue even when nothing consumes its output, which given the preview cost noted on :457 is not free.
All four existing users of has_intermediate_output set it without is_output_node: nodes_painter.py:31, nodes_curve.py:15, nodes_glsl.py:704, nodes_images.py:69. This is the only place in the repo that combines them, and those four are the closest analogues (canvas-editor nodes with a live UI region).
Might well be deliberate, since the editor does need a result to open against. If so it is worth a one-line comment saying why, because the next person reading it against those four will assume it is a copy-paste artefact.
christian-byrne
left a comment
christian-byrne
left a comment
There was a problem hiding this comment.
Reviewed at 3b3c61dd. One ask before merge that I think is worth blocking on, and it is a handful of lines. Two smaller ones inline below.
Layers.LayerItem cannot express what the editor already produces
At head, LayerItem carries x, y, flip_h, flip_v — but no rotation and no scale (w/h). The unversioned COMPOSITOR blob it is meant to supersede carries a full affine transform, and nodes_compositor.py reads every part of it: rotation at :301, applies it at :326-328, and w/h feed placed_bounds at :390. The bridge between the two hardcodes the loss — state_from_items writes "rotation": 0 at :163 because LayerItem has nowhere to put the real value.
So the new versioned public type is a strict subset of the unversioned widget blob it is meant to replace.
The consequence is not cosmetic. It means COMPOSITOR cannot be retired: anything that round-trips through LAYERS loses rotation and scale, so the widget blob has to stay as the only representation that carries them. We end up maintaining two coexisting layer representations indefinitely — permanent by accident rather than by decision. Adding the fields now is the thing that keeps that a choice rather than a fait accompli.
LAYERS is a public comfy_api io type. Adding fields to it after it ships is a breaking change for every custom node that builds one; adding them now is free. flip_h/flip_v already made it in, so the transform is nearly complete — this closes it:
class LayerItem(TypedDict):
...
flip_h: NotRequired[bool]
flip_v: NotRequired[bool]
rotation: NotRequired[float] # radians, matching the editor's Transform.rotation
w: NotRequired[int] # or a single scale: NotRequired[float]
h: NotRequired[int]Whichever spelling matches what the editor already emits is the right one — AddLayer can default them and state_from_items can stop hardcoding 0. This does not need to hold up anything else in the PR; it is just far cheaper today than after the type is public.
| flip_h: NotRequired[bool] | ||
| flip_v: NotRequired[bool] | ||
| color: NotRequired[str] |
There was a problem hiding this comment.
rotation and scale are the two fields missing here, and they are the two the editor lets users set. Full argument in the review body; the short version is that nodes_compositor.py:301,326-328,390 all read them off the COMPOSITOR blob, and state_from_items:163 hardcodes "rotation": 0 because there is nowhere in LayerItem to put it.
| flip_h: NotRequired[bool] | |
| flip_v: NotRequired[bool] | |
| color: NotRequired[str] | |
| flip_h: NotRequired[bool] | |
| flip_v: NotRequired[bool] | |
| rotation: NotRequired[float] | |
| w: NotRequired[int] | |
| h: NotRequired[int] | |
| color: NotRequired[str] |
| def canvas_extent(frames: list[dict]) -> tuple[int, int]: | ||
| return ( | ||
| max(frame["x"] + frame["tensor"].shape[2] for frame in frames), | ||
| max(frame["y"] + frame["tensor"].shape[1] for frame in frames), | ||
| ) |
There was a problem hiding this comment.
Retracting most of this — I duplicated two threads that are already open on this file, and I should have checked first.
Both points I made here are already covered, in more detail than I had:
- the negative-
xcanvas collapse: [Partner Nodes] feat: ImageCompositor node with layer-state compositing, layer from bbox and Seedream Layer Separation node #15317 (comment) _numberacceptingnan/inf(plus unboundedw/h): [Partner Nodes] feat: ImageCompositor node with layer-state compositing, layer from bbox and Seedream Layer Separation node #15317 (comment)
Please treat those two as the live threads and ignore the duplicate detail below. The only thing worth adding, and the reason I am leaving this comment rather than deleting it:
The max-extent-of-placed-layers approach was our suggestion, so the missing lower bound is on us, not on you. It is worth saying that out loud rather than filing it as your bug. The max(1, ...) clamp is the deadline-safe half; the "a layer at negative x is silently cropped and never widens the canvas" half is the part that actually needs a design call, and it is already laid out on the thread linked above.
Nothing else here is new — the review body above (Layers.LayerItem has no rotation and no scale) is the only finding in this round that is not already on the PR.
Adapter node: IMAGE (batch) + BOUNDING_BOX (+ MASK, + LAYERS) -> LAYERS. One document item per frame, each placed by its own box. Why this is needed. A node that separates an image into elements emits them as an image batch plus a list of boxes. That batch cannot be fed to Create Layered Image with the placement intact, because `expand_item_frames` applies the item's single x/y/w/h/name/z_index to every frame of a batch. Sixteen layers get one placement between them. The current workaround is to have the producer pre-place each layer on a full-size canvas so x=0,y=0 is correct for all of them. That works but is expensive: at 2K with 16 layers it carries ~768MB of layer tensors and ~268MB of masks to encode what cropped layers hold in a fraction of it. This node emits one item per layer instead, so each carries its own placement. It needs no change to the existing compositor path: `document_items` already sorts by z_index, and `expand_item_frames` already handles single-frame items correctly - the shared-placement limitation only bites on batches. Reads `metadata.name`, `metadata.z_index` and `metadata.content_rect` where present. `crop_to_content` trims each frame out of a padded batch and places it by its box, which is what recovers the memory win. Also restores a `_bbox_list` parser. The equivalent (`_bbox_entries`, `layout_bboxes`, `state_from_bboxes`) was removed in 1c4953f along with the rest of the bbox handling when the node moved to the LAYERS document, so there is currently no path from a bounding box into the compositor. Verified: two padded layers with different content sizes, cropped and placed independently, composite to within 0.50/255 of expectation - the 8-bit quantisation floor of the PIL round trip inside the compositor. The 76 existing compositor tests still pass.
- crop metadata.content_rect at its actual region instead of the top-left corner - resolve normalized element boxes via boxes_from_input, requiring canvas dims - accept BOUNDING_BOX, Array or JSON string like the original bboxes input - scale layers to their box via item w/h and name layers from metadata desc
Signed-off-by: bigcat88 <bigcat88@icloud.com>
metadata.content_rect is frame-relative, so the crop lands at box x/y plus the rect origin - covering both padded-to-canvas frames (box at the origin) and tight-cropped frames (placement carried by the box).
9821461 to
ef431a4
Compare
jtydhr88
changed the title
… dropping the background entry Signed-off-by: bigcat88 <bigcat88@icloud.com>
bigcat88
changed the title
bigcat88
left a comment
bigcat88
left a comment
There was a problem hiding this comment.
The code related to the Partner Node has been verified and is solid. From a usability standpoint, the new Core Node is also in good shape and was extensively tested.
may replaced by #14821 ## Summary - Layer editor engine ported from pentrado (document model, WebGL2 compositor, render stack, select/transform tools, undo/redo commands) under src/core/layerEditor - Full-screen Compositor dialog: layer list with fixed solid-color background layer, figma-style transform gizmo, context properties panel (position/dimensions/alignment/rotation/flip/opacity/blend, canvas dimensions + background fill), top toolbar pill, PSD export via lazy-loaded ag-psd - Batch-image entry point on node image previews - ImageCompositor node frontend: autogrow image inputs, composite preview widget, and a first-class object-valued COMPOSITOR widget holding the layer_state v2 recipe (transform/opacity/blend/flip + background + input fingerprints); upstream input changes reset the node to its default state and the editor re-initializes from the optional bboxes layout forwarded via ui.compositor_bboxes - Instant node preview after save via a local per-node preview override; saving writes only the recipe, no file uploads BE is Comfy-Org/ComfyUI#15317 ## Screenshots (if applicable) <img width="2860" height="1421" alt="image" src="https://github.com/user-attachments/assets/1b551c00-15e8-43f4-87cf-87d9833bbe9f" /> <img width="3500" height="1684" alt="image" src="https://github.com/user-attachments/assets/991dc022-45a4-40da-9a16-1a1b04280571" /> <img width="2912" height="1476" alt="image" src="https://github.com/user-attachments/assets/e7d417b9-5fb4-436e-9907-3c30bbad2ea3" /> --------- Co-authored-by: Pablo <wablomann@gmail.com> Co-authored-by: PabloWiedemann <PabloWiedemann@users.noreply.github.com> Co-authored-by: Christian Byrne <cbyrne@comfy.org> Co-authored-by: Connor Byrne <c.byrne@comfy.org>
244635e
5 participants
FE needs Comfy-Org/ComfyUI_frontend#14809
screenshot
This node is best used in conjunction with Create Layered Image from #15317 — the
layer_stackoutput connects to it directly with a single wire.API Node PR Checklist
Scope
Pricing & Billing
If Need pricing update:
QA
Comms