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 load 3d node support #5564

Merged
merged 2 commits into from
Dec 13, 2024
Merged

Conversation

jtydhr88
Copy link
Contributor

@jtydhr88 jtydhr88 commented Nov 10, 2024

Hi,
I implment a node to load local 3d models, here is FE change:
Comfy-Org/ComfyUI_frontend#1563

which needs this BE changes.

here are the introduction video as your reference:

  1. basic load feature
1.load-3d-intro-width-height.mp4
  1. showGrid, cameraType, View
2.load-3d-intro-showGrid-cameraType-View.mp4
  1. material, lightIntensity
3.load-3d-intro-material-lightIntensity.mp4
  1. fbx and animation
4.load-3d-intro-fbx-animation.mp4
  1. up-direction
5.load-3d-intro-param-up-direction.mp4
  1. multi-models (video file is too big)
  2. supported-formats (video file is too big)
  3. preview-3d-node
8.preview-3d-node-intro.mp4

huchenlei
huchenlei previously approved these changes Nov 19, 2024
@huchenlei huchenlei added the Good PR This PR looks good to go, it needs comfy's final review. label Nov 19, 2024
@huchenlei
Copy link
Collaborator

@comfyanonymous PTAL, the frontend PRs are already merged. We should have this PR merged on next frontend sync this Friday.

@yovsac
Copy link

yovsac commented Dec 4, 2024

This node can be a revolution on how we use comfy. Is it going to be implemented any time soon??

@huchenlei
Copy link
Collaborator

@comfyanonymous PTAL. I do think @jtydhr88 should become the owner of these nodes, i.e. we don't try to understand every detail of the implementation and when issue pop-up we just do necessary reverts and assign issue to the owner.

return output_image, output_mask, model_file,

class Preview3D():
@classmethod
Copy link
Owner

Choose a reason for hiding this comment

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

Does this node need to be in the backend? Can it be a frontend only node?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, I think so.
let me check and give you back soon

Copy link
Collaborator

Choose a reason for hiding this comment

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

good catch, I think so. let me check and give you back soon

Let's just remove that Preview3D node for now and get this PR merged. We can decide how to handle that Preview3D node later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed, I will work on how to implement Preview3D on FE only

@comfyanonymous comfyanonymous merged commit bdf3937 into comfyanonymous:master Dec 13, 2024
5 checks passed
@yovsac
Copy link

yovsac commented Dec 13, 2024

Great news! Does this mean we can start using it when we update comfy ?

@AndyWu2015
Copy link

so cool, just want to try comfyui. thanks Terry. (太酷啦,正好想试着用用comfyui。感谢Terry)

@jtydhr88
Copy link
Contributor Author

Great news! Does this mean we can start using it when we update comfy ?

Yes! please update to the latest!

@yovsac
Copy link

yovsac commented Dec 14, 2024

Great thanks!!!

@jtydhr88 jtydhr88 mentioned this pull request Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good PR This PR looks good to go, it needs comfy's final review. Important
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants