Skip to content

feat: add Preview3DAdvanced node#14175

Open
jtydhr88 wants to merge 1 commit into
masterfrom
feat/preview3d-advanced
Open

feat: add Preview3DAdvanced node#14175
jtydhr88 wants to merge 1 commit into
masterfrom
feat/preview3d-advanced

Conversation

@jtydhr88
Copy link
Copy Markdown
Contributor

@jtydhr88 jtydhr88 commented May 29, 2026

A new Preview3D that emits live viewport state as outputs: model_file, camera_info, model_info, width, height. Reuses the Load3D capture widget and the PreviewUI3D side effect.
Input model_file is File3D-only (no String path) to keep the contract typed; width/height are pre-promoted to socket inputs.

Inputs win when connected; otherwise the viewport's live framing & transform carries through.
The same resolved values flow to the output (so chained Preview3DAdvanced nodes propagate in a single run, and headless execution still carries state) and to PreviewUI3DAdvanced (so the viewport applies the input when one is connected).

FE: Comfy-Org/ComfyUI_frontend#12527

2026-05-29.22-57-21.mp4

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 29, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR introduces Preview3DAdvanced, a new experimental output node for handling 3D file previews. The node accepts a 3D model file, input image, optional camera parameters, and model metadata, along with configurable output dimensions. During execution, it persists the 3D asset to the output directory under a generated filename and returns the original model file, camera information, model metadata, dimensions, and a UI preview instance. The node is registered in the extension's public node list.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding a new Preview3DAdvanced node, which is the primary focus of this PR.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description clearly describes the new Preview3DAdvanced node, its inputs/outputs, behavior, and design decisions.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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_load_3d.py`:
- Around line 152-177: The execute method in Load3D node (execute) currently
ignores the optional input sockets: it always reads camera_info and
model_3d_info from the widget-provided image dict, while the UI preview reads
kwargs.get("camera_info"), so wired inputs are dropped or inconsistent. Fix by
preferring connected inputs from kwargs when present: compute camera =
kwargs.get("camera_info", image.get("camera_info")) and model_3d =
kwargs.get("model_3d_info", image.get("model_3d_info", [])), use those same
values in the IO.NodeOutput and pass camera to UI.PreviewUI3D so the output and
preview are consistent and wired inputs are respected; alternatively remove the
unused input sockets if you intend to always use widget values.
- Around line 138-150: Preview3DAdvanced.execute() currently ignores the
explicit inputs IO.Load3DCamera.Input("camera_info") and
IO.Load3DModelInfo.Input("model_3d_info") and always returns
image['camera_info'] and image.get('model_3d_info', []); update execute() in
Preview3DAdvanced to prefer the provided inputs (kwargs.get("camera_info") and
kwargs.get("model_3d_info")) and fall back to values embedded in the loaded
image (image.get('camera_info') and image.get('model_3d_info', [])), or remove
the unused input nodes if you intend the image fields to be authoritative;
reference Preview3DAdvanced, its execute() method,
IO.Load3DCamera.Input("camera_info"), IO.Load3DModelInfo.Input("model_3d_info")
and IO.MultiType.Input("model_file", ...) when making the change.
🪄 Autofix (Beta)

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: CHILL

Plan: Pro

Run ID: 908ae6a9-4055-46dc-9023-9d17641404cd

📥 Commits

Reviewing files that changed from the base of the PR and between ec1896a and eb7cd3b.

📒 Files selected for processing (1)
  • comfy_extras/nodes_load_3d.py

Comment thread comfy_extras/nodes_load_3d.py
Comment thread comfy_extras/nodes_load_3d.py
@jtydhr88 jtydhr88 force-pushed the feat/preview3d-advanced branch from eb7cd3b to 97224c9 Compare May 30, 2026 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants