donmccurdy / glTF-Transform

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

optimize removes leaf nodes with --prune-leaves false #1426

Closed bobbens closed 3 days ago

bobbens commented 5 months ago

Describe the bug

gltf-transform optimize --prune false/gltf-transform optimize --prune-leaves false removes leaf nodes from a gltf, while gltf-transform prune --keep-leaves true works as expected and keeps the leaf nodes (although without --keep-leaves true in prunes them).

To Reproduce Steps to reproduce the behavior:

  1. Create the following GLTF file as /tmp/test.gltf:
    {
    "asset":{
      "generator":"Khronos glTF Blender I/O v4.1.63",
      "version":"2.0"
    },
    "scene":0,
    "scenes":[
      {
         "name":"base",
         "nodes":[
            0
         ]
      }
    ],
    "nodes":[
      {
         "extras":{
            "foo":"bar"
         },
         "name":"test",
         "translation":[
            -20.5,
            -3,
            -9
         ]
      }
    ]
    }
  2. Run gltf-transform optimize --prune-leaves false /tmp/test.gltf /tmp/out.gltf
  3. Notice the nodes with a single node that exists the GLTF file and consist of a translation with an "extras" field are missing in /tmp/out.gltf.

Expected behavior The leaf nodes should be kept, as done by gltf-transform prune --keep-leaves true /tmp/test.gltf /tmp/out.gltf

Versions:

Additional context The following command gltf-transform optimize --prune-leaves false --flatten false --weld false --simplify false --join false /tmp/test.gltf /tmp/out.gltf does work, however, it disables too much functionality.

donmccurdy commented 5 months ago

Hi @bobbens!

Part of the issue here is that --flatten (on by default) also removes leaf nodes after it has finished simplifying the scene graph. That's part of a larger limitation, related to https://github.com/donmccurdy/glTF-Transform/issues/1278, that the cleanup steps in various optimizations aren't tightly coupled to the changes actually made by those optimization steps.

I would have expected that --no-flatten --prune-leaves false would have been enough to prevent this, but I see that's not the case, and it will take some more investigation to see why that is.

The 'optimize' command takes the "kitchen sink" approach here; joining meshes, instancing meshes, and flattening the scene graph might all be difficult to square with preserving a particular empty node somewhere in that scene graph. So, I'll keep this issue open, but you may find it easier to pick and choose the optimizations you want, either with individual CLI commands, a custom CLI config, or the scripting interface.

bobbens commented 5 months ago

Thank you for the suggestions. I would assume --prune-leaves false would not remove leaves, so potentially have it also implicitly apply --no-flatten would make sense, assuming --no-flatten --prune-leaves false wouldn't remove the leaf nodes as it does now.

I would like to keep on using the "kitchen sink", as this is part of a blender -> gltf production chain, and it seems like it should be lower maintenance in the long run compared with having to modify or edit scripts to update functionality. I'll try to wait for a resolution as I'm not necessarily in a hurry at the moment.

Thanks for the good work on gltf-transform!