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

Running 'instance()' creates incorrect transforms on some nodes #1505

Closed
donmccurdy opened this issue Sep 18, 2024 · 2 comments · Fixed by #1507
Closed

Running 'instance()' creates incorrect transforms on some nodes #1505

donmccurdy opened this issue Sep 18, 2024 · 2 comments · Fixed by #1507
Labels
bug Something isn't working package:functions
Milestone

Comments

@donmccurdy
Copy link
Owner

Describe the bug

Originally reported at pmndrs/gltfjsx#277, running optimize on the attached models causes the window panels to be rotated 90º and out of position. Disabling instancing with --no-instance prevents the issue, which suggests that instance() may have a bug.

https://github.com/user-attachments/files/17029993/venice-transformed.zip

To Reproduce

# ❌ broken
gltf-transform optimize venice.glb --no-compress

# ✅ ok
gltf-transform optimize venice.glb --no-compress --no-instance

Expected behavior

Visual consistency in the instanced model.

Versions:

  • Version: v4.0.8
  • Environment: Node.js
@donmccurdy donmccurdy added bug Something isn't working package:functions labels Sep 18, 2024
@donmccurdy donmccurdy added this to the v4.0 milestone Sep 18, 2024
@zeux
Copy link

zeux commented Sep 20, 2024

Likely unrelated to the transform issue, but also want to note that in the transformed glb, at least one node (Window.021.0) has no mesh but has a EXT_mesh_gpu_instancing extension with some number of instances. I flagged this in KhronosGroup/glTF#2404 but that PR did not reach a resolution so I still don't know if this is supposed to be valid or not. At the very least it seems non-optimal wrt output file size.

@donmccurdy
Copy link
Owner Author

donmccurdy commented Sep 22, 2024

Thanks @zeux! It was indeed related. The original file already used EXT_mesh_gpu_instancing, and the implementation of the instance() function was not prepared for that, detaching the existing meshes but leaving EXT_mesh_gpu_instancing extensions behind. Will be fixed by #1507.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working package:functions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants