fix(IfcImporter): add vertex key to geometry dedup hash

[ShaMan123]

Description

closes #237

Additional context

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following:

• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Follow the Conventional Commits v1.0.0 standard for PR naming (e.g. feat(examples): add hello-world example).
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[ShaMan123]

The issue references an concise ifc repro fixture that can be used as a test case (once vitest is in)

[ShaMan123]

The vertex key construction is Claude's suggestion.

[ShaMan123]

Actually I am not sure this is a correct fix.
This doesn't cover cases that are geometrically the same with differently ordered vertices.

[agviegas]

Great repro, it makes the problem obvious: both plates share outline, area, volume and centroid, and the only difference is where the holes sit, which is exactly what the old key never looked at.

Direction is right. One thing to fix first: the vertexKey sum can cancel. JS % keeps the sign of the dividend, so h goes negative for negative coordinates and stays signed through the mixing. Summed commutatively, two vertices whose hashes are exact negatives cancel to zero and drop out of the key. The final if (vertexKey < 0) only fixes the sign of the result, not the cancellation. Normalize each h first:

let h = Math.round(position[i] * p) % MODULUS;
if (h < 0) h += MODULUS;

And a question: does the key need to be order-invariant at all? The commutative sum buys a match on a permuted buffer but costs collision resistance, where an ordered polynomial hash is stronger and simpler. Do we actually see web-ifc emit permuted duplicates? If not, I'd take the ordered one.

Minor: the AABB doesn't separate your two plates (same outline, same box), so vertexKey is carrying the fix alone. Fine to keep the corners, just noting what's load-bearing.

The #237 repro would make a good regression test. Still a draft, what's left on your side?

[ShaMan123]

I am not an expert of hashing so I do not feel comfortable suggesting the correct solution. I do understand and agree that the current is lacking due to sign cancellation (part of why the PR is a draft).
That being said:

And a question: does the key need to be order-invariant at all? The commutative sum buys a match on a permuted buffer but costs collision resistance, where an ordered polynomial hash is stronger and simpler. Do we actually see web-ifc emit permuted duplicates? If not, I'd take the ordered one.

I am not sure how web-ifc handles it. I am very cautious due to the last month (working on Tekla garbage ifc dedup). I would argue that we could add a simple sort on the holes before hashing and that will cover all cases.

Minor: the AABB doesn't separate your two plates (same outline, same box), so vertexKey is carrying the fix alone. Fine to keep the corners, just noting what's load-bearing.

You mean that the AABB isn't needed any longer since the vertex key handles it? Is there no other case that relies on the AABB?

[agviegas]

Ran it through web-ifc 0.0.77 so we stop guessing. Three cases, reading the exact vertex buffer the dedup key sees:

1. Two independent facesets, identical authoring: identical vertex order. Same geometry in, same buffer out.
2. Same box, point list reversed and faces reindexed to match (same face order): still identical order. Reindexing points alone doesn't permute the output.
3. Same box, same points, face list in a different order: same vertex set, permuted order.

So web-ifc never spontaneously permutes, but it hands back a permuted buffer when the source lists the same triangles in a different order (case 3). It emits in face-iteration order, so triangle order in the IFC is what leaks through.

What that means for the key. The common duplicate case (an exporter repeating a product, or a shared representation map / mapped items) is case 1, byte-identical, so an ordered polynomial hash over the buffer dedups it, and it drops the sign-cancellation problem for free since you never sum commutatively. The only thing ordered misses is case 3: geometrically identical but triangle-order-permuted, which is what your commutative sum buys today. Dropping to ordered would stop deduping those, a memory miss, not wrong geometry.

So the real decision is whether case 3 is worth chasing. If yes, don't keep the commutative sum (weak collisions plus the cancellation trap). Do it order-invariant the robust way: hash each vertex to a canonical value, sort those hashes, then fold them with an ordered polynomial. That is where a sort belongs, over all per-vertex hashes, not over holes, since case 3 shows the permutation is whole-buffer triangle order with no separable holes list to sort.

My read: case 3 needs two products that are the same shape but authored with different triangle order, which a single exporter rarely does within one file. I would ship the ordered hash, which fixes #237 and is simpler and stronger, and add the sorted order-invariant variant only if a real file actually shows triangle-order-permuted duplicates. Happy to go straight to the sorted version if your Tekla files make you want belt and braces, it is just the extra O(n log n) sort.

On performance: no meaningful cost either way. The vertex-key loop runs once per distinct geometry, not per instance (the _previousGeometriesIDs check short-circuits repeated representations before any buffer work), and it can be fused into the unit-scaling loop that already walks every coordinate, so it is zero extra passes. web-ifc's tessellation dominates. The only variant that actually costs conversion time is the sorted one, because of the per-geometry sort, which is the other reason I would default to ordered.

On the AABB: keep it, I wasn't calling it redundant. In your repro the two plates share the same box, so there the vertexKey is what separates them and the box isn't doing the separating. In general the box is a cheap early discriminator that rejects differently sized geometry before the per-vertex loop runs, and it is independent signal from the fold. Worth keeping.