Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add preview 3d node #6070

Merged
merged 2 commits into from
Dec 17, 2024
Merged

Conversation

jtydhr88
Copy link
Contributor

@jtydhr88 jtydhr88 commented Dec 16, 2024

Even we discussed in #5564 that should move this preview 3d into FE, however after investigated, I could not figure out how to do that, the main reason is when execute workflow, comfyUI will validate note mapping here

class_ = nodes.NODE_CLASS_MAPPINGS.get(class_type, None)
but the pure FE node doesn't have such Python Class, I believe we still need one BE node here.
Here is a demo video

2024-12-15.22-54-45.mp4

FE PR is Comfy-Org/ComfyUI_frontend#1918

huchenlei
huchenlei previously approved these changes Dec 16, 2024
@huchenlei
Copy link
Collaborator

Note: currently model_file is a string path. We probably want to define a 3D data type in ComfyUI? @jtydhr88 Can you help define a universal 3D data type that is easy to handle?

@jtydhr88
Copy link
Contributor Author

Note: currently model_file is a string path. We probably want to define a 3D data type in ComfyUI? @jtydhr88 Can you help define a universal 3D data type that is easy to handle?

Sure, I will think abou it

@jtydhr88
Copy link
Contributor Author

@huchenlei one test (regarding hunyuan node?) failed but I don't see any relevent to my changes.

@jtydhr88 jtydhr88 requested a review from huchenlei December 17, 2024 00:46
@huchenlei
Copy link
Collaborator

@huchenlei one test (regarding hunyuan node?) failed but I don't see any relevent to my changes.

Can you rebase to the head of master? Missing imports are fixed in 0b25f47

@huchenlei huchenlei merged commit 517669a into comfyanonymous:master Dec 17, 2024
4 of 5 checks passed
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