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

chore(functions): Remove lossy weld #1357

Closed donmccurdy closed 7 months ago

donmccurdy commented 7 months ago

Avoids having to maintain two versions of weld(), the lossless version is vastly faster. Future updates to Meshoptimizer will include simplifyWithAttributes (#1335), which I hope will provide a better approach for joining vertices despite differences in vertex attributes, and allowing weld() to remain a faster, more focused, and more maintainable method.

I spent some time trying other approaches — like #1356, trying to consolidate the two methods. But that got complicated quickly. If I were trying that again, I think I'd take an approach similar to #1356 but have the QuantizedVertexStream class apply rounding when reading each vertex, rather than iterating the vertex attributes up front. Then we need to rebuild the 'writeMap' after removing degenerate triangles, write a temporary index to the geometry without those triangles, then run it through compactPrimitive... it's just messier.

But I'm hoping a lossless weld is enough — that's all gltfpack appears to do for example. And when #1335 is available, that will be a (likely better) alternative for merging vertices that are not identical.

zeux commented 7 months ago

fwiw currently simplifyWithAttributes will still treat topological discontinuities the same as simplify; attributes are currently used to reduce attribute+geometric error during simplification but don't affect any collapse restrictions. I have some long term plans to improve this but just noting that it's not going to be a drop-in replacement to "weld with tolerance + simplify" for some time. (is it a problem for gltf-transform? not sure! gltfpack doesn't do tolerance-based welding either)

donmccurdy commented 7 months ago

Thanks @zeux! My impression so far is that welding based on bitwise-equality before simplification works well, both here and in gltfpack, so I'm hopeful that glTF Transform's previous lossy welding was just unnecessary.

Another possible workaround — if anyone does want a more aggressive weld — would be to quantize before welding, like:

import { quantize, weld, simplify } from '@gltf-transform/functions';
import { MeshoptSimplifier } from 'meshoptimizer';

await MeshoptSimplifier.ready;

await document.transform(
  quantize({
    quantizePosition: 14,
    quantizeNormal: 8,
    quantizeColor: 8
  }),
  weld(),
  simplify({
    simplifier: MeshoptSimplifier,
    error: 0.001
  })
);

That would provide more control than the previous tolerance-based weld, which didn't expose as much as QuantizeOptions. I'll add something about that to the weld() documentation as well.

donmccurdy commented 7 months ago

Unfortunately, a simple counter-example to my idea:

  1. create the default 'monkey' mesh in blender
  2. subdivide it 5-6 times
  3. export the result to GLB

Here's that mesh, compressed to meet GitHub's size restrictions:

monkey_500k_vertices+draco.glb.zip

A lossless weld won't do anything to this model, and as a result simplification does almost nothing either – it works only where the vertices are mostly coplanar. Applying quantization first, as I suggested above, allows simplification to proceed successfully only if I reduce quantizationNormal bits to 4 (normally limited to the 8–16 range, made local changes to test).

I suspect 4-bit normals will cause quality issues, so perhaps this quantization does need to be included as a condition in welding, rather than as a pre-processing step.

marwie commented 5 months ago

Hello @donmccurdy I've just encountered this issue as well when running our mesh LOD generation - for testing I've been using this model

Previously I was able to increase the threshold for lower-res LOD level which produced decent simplifcation (with some errors but that was mostly OK since those meshes would only be displayed at a distance) - see image below

image

After updating to 4.x all our LOD meshes are discarded unfortunately since weld+simplify does never exceed our simplification threshold of 5%

Using gltf-transform 3 I can specify the tolerance and increase it to 0.001 for example where simplify is reducing a decent amount of vertices.

I will look into adding a quantize op now and will reply here again :)

-- Update

Unfortunately using quantize isn't an option for me right now because of a non exported quantizePrimitive method - I need to be able to quantize a single primitive

donmccurdy commented 5 months ago

Probably we should try to export quantizePrimitive regardless of this issue. I think the API just needs a bit of cleanup first, to something like...

import { quantizePrimitive } from '@gltf-transform/functions';

quantizePrimitive(prim, transform /* mat4 */, options /* QuantizeOptions | undefined */);

But, I don't think that will be a complete fix for more aggressive welding and simplification. The previous lossy weld implementation was very slow, and a completely different code path from the newer one (inspired by Meshoptimizer), so I'd prefer not to bring that back as-is. Instead I think the best approach would be to extend the VertexStream implementation here...

https://github.com/donmccurdy/glTF-Transform/blob/fc3da02a969921c341a81f10f75d7aded53725b5/packages/functions/src/hash-table.ts#L50-L59

... with a lossyEquals function that compares attributes based on the configured per-attribute precision, or perhaps some different precision criteria. It will be slower than the lossless mode, but hopefully not as slow as the old code.

After that, we'll need a bit of extra work to clean up degenerate triangles and remove the primitive entirely if it collapses, which can't happen in the lossless mode. I don't think weldPrimitive needs to handle that cleanup, but weld() probably should.