donmccurdy / glTF-Transform

glTF 2.0 SDK for JavaScript and TypeScript, on Web and Node.js.
https://gltf-transform.dev
MIT License
1.43k stars 150 forks source link

Object transforms look off after using Instance #1509

Closed Kyozon closed 1 month ago

Kyozon commented 2 months ago

Describe the bug The resulting .gltf scene looks off when using the gltf-transform instance.

To Reproduce Steps to reproduce the behavior:

  1. Download: merged.zip

  2. Import merged.gltf into Blender and verify original transforms as seen in the image below:

Default

  1. Use gltf-transform instance merged.gltf instanced.gltf

  2. Import instanced.gltf into Blender and verify the bug as seen in the image below:

Instance

Expected behavior Retain the original transforms but optimize the scene to use GPU instances.

Versions:

donmccurdy commented 2 months ago

Hi @Kyozon! The instanced scene looks correct to me, both in Blender and in https://gltf-viewer.donmccurdy.com/. Could you confirm if you're seeing the problem both in Blender and in my viewer, and perhaps share the 'broken' copy of the model as well?

CleanShot 2024-09-24 at 16 40 19@2x

Kyozon commented 2 months ago

Hi @donmccurdy. Thank you for the quick reply!

Sure, here is the broken file: Instanced.zip

My filed failed to open in your viewer, here is the error message:

Validation

donmccurdy commented 2 months ago

Hm... something is weird in the source file:

  "buffers": [
    {
      "uri": "glTF.bin",
      "byteLength": 8160
    },
    {
      "uri": "glTF.bin",
      "byteLength": 8160
    },
    {
      "uri": "glTF.bin",
      "byteLength": 8160
    }
  ],

Three buffers, all pointing at the same thing... I'm not sure whether it's technically valid in the glTF spec, but the pygltflib authors probably didn't intend that, and it's leading to data loss here.

I'll have to think a bit about the best way to handle it, but in the meantime, you can avoid the issue by writing to .glb instead of .gltf:

gltf-transform instance merged.gltf instanced.glb
donmccurdy commented 1 month ago

Fixed in #1511. With these changes glTF Transform will log a warning (Duplicate resource URI, "glTF.bin") but will continue to process the file. When writing to .gltf, the additional buffers will be written as separate files with unique names. In this case:

To consolidate the binary data and avoid extra files, either write to .glb or use the unpartition() function.

instanced 3D letters, no bugs

donmccurdy commented 1 month ago

Reverting #1511 in https://github.com/donmccurdy/glTF-Transform/commit/84e77f58d321702dba486fc07ed1ce7279d4897d and reopening the issue. Original approach to fixing the issue will cause other problems.

donmccurdy commented 1 month ago

New attempted change in #1522. Unfortunately not a "fix", but will throw a clear error if conflicting buffer URIs are going to break the file. To workaround it will still be necessary to write to .glb or use the unpartition() function.