Simplify breaks due to assertion failed

[wayne-wu]

Describe the bug

I'm running into a bug where gltf-transform simplify from the command line breaks with an assertion failed error.

Upon closer examination, it looks like this error is coming from this line here (from the meshopt simplify function):
https://github.com/zeux/meshoptimizer/blob/master/js/meshopt_simplifier.js#L178 , where targetCount > indices.Length after one iteration therefore failing the assertion.

One thing I did notice from the example of simplify here: https://github.com/zeux/meshoptimizer/blob/master/demo/simplify.html -- it looks like the targetCount is calculated based on index count and not the vertex position count as used by gltf-transform. I'm wondering if this is intended, and whether that relates to the bug?

To Reproduce
I'm unable to share the model I was testing with, but I'll see if I can recreate the bug with another model.

Versions:

• Version: cli.js 4.0.0-alpha.5 — Command-line interface (CLI) for the glTF Transform SDK.
• Environment: Command line

Thanks!

[donmccurdy]

Thanks @wayne-wu! Index count vs. vertex count sounds like a good guess on the cause. Perhaps happening when the source vertex attributes have many vertices, but the indices only use a smaller subset of those. The relevant code is here:

glTF-Transform/packages/functions/src/simplify.ts

Lines 162 to 170 in 3901a3e

	const targetCount = Math.floor((options.ratio * srcVertexCount) / 3) * 3;
	const [dstIndicesArray, error] = simplifier.simplify(
	indicesArray as Uint32Array,
	positionArray as Float32Array,
	3,
	targetCount,
	options.error,
	options.lockBorder ? ['LockBorder'] : [],
	);

I'll look into reproducing the issue without the model, but if you'd like to try making the change and then testing on the model yourself, it would look something like:

yarn install
yarn dist
node packages/cli/bin/cli.js simplify in.glb out.glb

Or yarn watch will continuously watch and rebuild the code as you're making changes.

[donmccurdy]

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.

[harrycollin]

I encountered this using @gltf-transform/functions@4.0.0-alpha.15 package in node.js.

[donmccurdy]

@harrycollin possible to share a reproduction? The tests I have are passing. Meshoptimizer may have assertions for any number of things.

[harrycollin]

My mistake! It looks like I was passing undefined values to the ratio, error and lockBorder options.

[donmccurdy]

Ah, well that's a footgun! Should be fixed with:

• feat(functions): Add assignDefaults() #1382