ComfyUI-3D-Pack icon indicating copy to clipboard operation
ComfyUI-3D-Pack copied to clipboard

Branch trellis update

Open iiiCpu opened this issue 1 year ago • 5 comments

I had a problem with original Trellis pipeline on Windows 11 (wasted like 6 hours to make it work with no effect). So, I replaced it with Microsoft's one, which now works like charm. For me, I had to launch microsoft/TRELLIS app.py, let it build all it needed to build, copy all microsoft/TRELLIS/trellis contents into ComfyUI-3D-Pack/Gen_3D_Modules/TRELLIS/trellis.

Also, I had to move some mesh postprocess code from trellis.postprocessing_utils into Trellis_Structured_3D_Latents_Models.save_gs as I wanted to get MESH instead of just saved file name.

I also changed Save_3D* nodes to act more like standard Save_Image node: to add index to a file name instead of replacing it. And I also made option to create entire folder, it's useful to save different formats or preview into the same place.

iiiCpu avatar Jan 12 '25 10:01 iiiCpu

Great job! A small question, maybe trellis_ folder is unnecessary?

YanWenKun avatar Jan 20 '25 17:01 YanWenKun

A small question, maybe trellis_ folder is unnecessary?

True.

I also made a little clean up in nodes.py.

iiiCpu avatar Jan 21 '25 01:01 iiiCpu

I just tested this PR and found some files that would be better left unchanged:

  1. renderers/gaussian_render.py

    3D-Pack uses diff-gaussian-rasterization from ashawkey, which is incompatible with the version used by the TRELLIS official repo. Other parts of 3D-Pack are using this package as well, so it's better to stay with previous implementation.

  2. modules/attention/__init__.py

  3. modules/sparse/__init__.py

    3D-Pack uses xformers by default to ensure maximum compatibility. Replacing it with flash_attn would require providing additional options for users to choose from.

  4. representations/mesh/flexicubes/examples

    I believe these example files are not needed in this use case.

In addition, modifying the Save 3D Mesh node will break all existing workflows. I suggest creating a new node and including it in a separate pull request.

YanWenKun avatar Jan 21 '25 21:01 YanWenKun

  1. Agreed.

  2. I simply replaced Trellis from official Microsoft repo, without much thinking. I don't know much about xformers and flash_attn compatibility. Seems like I have some homework to do.

  3. Agreed.

p.s. Gosh, markdown sometimes make me feel so miserable.

iiiCpu avatar Jan 21 '25 22:01 iiiCpu

@iiiCpu Hi, thank you for your contribution, I'll take a closer look later this week In the meantime, can you please remove some unnecessary changes and change it back to xformers, because xformers actually supports flash_attn (Although probably not v3, but in time it should), thanks again in advance :) Have a good day 👍

MrForExample avatar Jan 22 '25 22:01 MrForExample