donmccurdy/glTF-Transform

Broken model after using instance transform

harrycollin opened this issue · 14 comments

Using the instance transform on the linked model results in misplaced objects. First discovered in Node and reproduced on https://gltf.report.

To Reproduce
Steps to reproduce the behavior:

  1. Go to Sketchfab Model
  2. Download the model as gLTF (glb)
  3. Go to https://gltf.report
  4. Open the downloaded model
  5. Use the following code and apply to the model
import { prune, dedup, resample, instance } from '@gltf-transform/functions';

await document.transform(
	// Remove duplicate vertex or texture data, if any.
	dedup(),

	instance(),

	// Losslessly resample animation frames.
	resample(),

	// Remove unused nodes, textures, or other data.
	prune(),
);

6: The resulting model looks like this:
image

Versions:

  • Version: 3.5.1
  • Environment: Browser, Nodejs

Additional context
This doesn't seem to happen via the CLI for me.

Confirmed, thanks @harrycollin! I'm seeing the same issue in the latest stable version v3.10.

I've been able to reproduce this error in https://gltf.report, but I can't reproduce it in a Node.js script or in the CLI, and it's fairly difficult to debug in the web app. if you're able to observe the error in Node, do you mind sharing how you went about that? Things that might be important:

  • Node.js version
  • ES Modules or CommonJS
  • TypeScript or JavaScript
  • any build tools, and config files if applicable

Thanks!

Note to self — broken version of the file contains no accessors in the instance TRS transforms.

Correct:

    {
      "mesh": 30,
      "extensions": {
        "EXT_mesh_gpu_instancing": {
          "attributes": {
            "TRANSLATION": 0,
            "ROTATION": 1,
            "SCALE": 2
          }
        }
      }
    },

Broken:

        {
            "mesh": 30,
            "extensions": {
                "EXT_mesh_gpu_instancing": {
                    "attributes": {}
                }
            }
        },

Thanks for looking at this! I first noticed this in my Node environment. My bundled version of glTF-Transform is higher than that used on https://gltf.report.

Info:

  • glTF-Transform 3.10.0
  • Node 18.x
  • ES Modules
  • Typescript
  • Bundled with esbuild
  • Banner config for esbuild:
  import { fileURLToPath } from 'url';
  import * as path from 'path';
  import { createRequire as topLevelCreateRequire } from 'module';
  const require = topLevelCreateRequire(import.meta.url);
  const __filename = fileURLToPath(import.meta.url);
  const __dirname = path.dirname(__filename);

if you're able to observe the error in Node

I created a very simple node script but couldn't reproduce the problem. However in my bundled version (info above) the problem still occurs. I can't think of anything else right now that would cause this to happen. I'll keep digging in the meantime.

I have a hunch this might be a case of dual package hazard. Certain build configurations compiling ESM to CJS (for example) can cause two copies of a dependency to be loaded. If the @gltf-transform/functions package imports code from one copy of @gltf-transform/core but is given an input document from another copy, lots of unpredictable bugs can happen, like material instanceof Material returning false.

If that's the case, setting output to ESM in either your ESBuild or TypeScript configuration would likely fix it ... see #857 (reply in thread) for a similar bug and resolution.

Actually reporting the same issue

On this model in particular :
https://github.com/Samsy/glbs/blob/main/sample.glb

Using the command line CLI ( 3.4.0 ) to instance it results good :
https://github.com/Samsy/glbs/blob/main/sample-instanced-good.glb

Screenshot 2024-02-08 at 15 30 28

But using GLTF report script to instance dont show anything, I opened the file on threejs editor, and does not seem to be correct. ( also tested on nodejs packages, no luck )

https://github.com/Samsy/glbs/blob/main/sample-instanced-broken.glb

Screenshot 2024-02-08 at 15 30 50

I also noticed, flatten does not seem to work on GLTF-report scripts

Flatten using cli ( 3.4.0 ) :

Screenshot 2024-02-08 at 15 44 46

Flatten using GLTF report scripts :

Screenshot 2024-02-08 at 15 44 24

Would someone be able to share a Node.js script that reproduces this error, along with the specific bundlers and typescript configurations (if any) used to build and run that script? The script in the top post is fine, but I think the build processes compiling it are creating problems.

I'm fairly confident this has to do with dual package hazard. Presumably I can fix the issue in glTF Report, but if you're running into the same issues in your own code, your build process may also need a configuration change. Targeting ESM (whatever that means in your build system) tends to resolve this problem.

I've found the cause — partial fix in #1269, but there are a few pieces of the solution I still need to figure out before merging.

Doesn't look like build processes or dual package hazard were to blame.

Fixed and published to 4.0.0-alpha.6. If you'd like to test that fix, you can use the @next tag, like npm install @gltf-transform/cli@next. glTF Report will be updated after the v4 stable release, which I'm hoping will happen by the end of March.

The issue was related to document.clone() leaving the scene graph in a subtly broken state, so if you need a workaround in v3 in the meantime, avoiding document.clone() should sidestep the issue.

Thank you very much, I'm going to check this !

Thank you @donmccurdy. Sorry I missed your request above for more info. I'm not sure I can avoid using clone(), so I'll test in v4 :D

I tested the next cli, confirms flatten / dedup / instance work as expected

Is there a way to install the "@next" version with node js ?

@Samsy yes – if you're using the scripting API in a node.js environment you'd probably want:

npm install --save @gltf-transform/core@next @gltf-transform/extensions@next @gltf-transform/functions@next