Skip to content

Conversation

@jtydhr88
Copy link
Collaborator

@jtydhr88 jtydhr88 commented Feb 8, 2025

add a output preview screen for load3d, because at before, it is hard to know the output exactly.
also moved two params (fov and light intensity) from node panel to the scene.
here is the demo:
https://github.com/user-attachments/assets/401f7773-a219-47de-b529-864c83ba7370
need Comfy-Org/ComfyUI#6742 as BE change

┆Issue is synchronized with this Notion page by Unito

Copy link
Contributor

@huchenlei huchenlei left a comment

Choose a reason for hiding this comment

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

Is it possible we build the UI with Vue/PrimeVue here? We don't need to reinvent some basic components like slider and svg icons.

You can reference

renderDragPreview(container) {
const vnode = h(NodePreview, { nodeDef: node.data })
render(vnode, container)
return () => {
render(null, container)
}
},
for how to mount a vue component under a container element.

container.classList.add('comfy-load-3d')

const load3d = new Load3d(container)
const load3d = new Load3d(container, true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Prefer using kwargs here. A boolean true second param will make reader confused.

container.classList.add('comfy-load-3d-animation')

const load3d = new Load3dAnimation(container)
const load3d = new Load3dAnimation(container, true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Prefer using kwargs here. A boolean true second param will make reader confused.


constructor(
container: Element | HTMLElement,
createPreview: boolean = false
Copy link
Contributor

Choose a reason for hiding this comment

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

Prefer using kwargs here. A boolean true second param will make reader confused.

@jtydhr88
Copy link
Collaborator Author

jtydhr88 commented Feb 8, 2025

close it for now, I will use vue to re-write first

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