Fix tessellation issues causing extra or invalid geometry (#515, #556)

[r-silveira]

This PR fixes the long-standing tessellation issue in Xbim.Geometry where
faces generated by the XbimTesselator produced
incorrect triangles: multiple triangles sharing the same vertex
or producing extra geometry not present in the IFC source.

Root cause

The problem was caused by the absence of a CombineCallback in the call to
tess.Tessellate(). When the Tesselator creates new vertices during polygon merging
or hole stitching, it reuses the Data field of existing vertices. As a
result, new points received duplicated indices, producing degenerate or
visually distorted faces.

Fix

• Implemented a proper CombineCallback that creates a new ContourVertex
  with Data = -1, ensuring that these new vertices are later added to the
  XbimTriangulatedMesh via AddVertex().
• Verified that faces with multiple loops (holes) now triangulate correctly
  and no longer generate stray triangles or spikes.

Related issues

• Fixes Geometry problem with Revit IFC-file #515 — Geometry problem with Revit IFC-file
• Fixes Extra geometry after procedure Mesh (xbim.tessellator) #556 — Extra geometry after procedure Mesh (xbim.tessellator)

Impact

This change improves the robustness and correctness of tessellation for
IFC models imported from Revit and other authoring tools, particularly for
faces containing holes or complex boundaries.

Issue:

Fixed mesh:

My test model:

My test model:
LX000-Isolated_Test.zip

[andyward]

Hi @r-silveira thanks for the investigation. I'm going to ask @Ibrahim5aad to take a look at this. While your solution may fix this issue I'm not 100% it's addressing the root cause, given this callback mechanic was introduced a few months ago and #515 pre-dates that. It feels like this workaround may just be masking the underlying cause, and the callback is intended to be optional (as it's for optimising meshes). It's a useful start as it should help us track the lower level issue with Vertexes

[r-silveira]

I agree. I believe there might be an issue with how the inner contours are created: either something is being propagated incorrectly, or a vertex is being generated wrong. If I comment out that part (highlighted in red), the invalid triangles are no longer generated. Maybe this can help with the investigation.

File: Tess.cs

[r-silveira]

I was reviewing the code to understand it better, and I think the best solution, without using the callback, would be to set _data = -1 in GetIntersectData().

Here’s my reasoning: when tessellating faces with holes, the GetIntersectData method in Tess.Sweep.cs creates new intersection vertices, and VertexWeights calculates their interpolated positions. However, if no combine callback is provided, the vertex _data field remains uninitialized, which leads to invalid triangle indices.
The fix is to add isect._data = -1; in the else branch of GetIntersectData (right after the callback check). This marks intersection vertices so they’re correctly added to the mesh later when if (idx < 0) is triggered in TriangulateFaces, ensuring that all triangles reference valid vertex indices.

So, if the code looks like this (Sweep.cs):

private void GetIntersectData(MeshUtils.Vertex isect, MeshUtils.Vertex orgUp, MeshUtils.Vertex dstUp, MeshUtils.Vertex orgLo, MeshUtils.Vertex dstLo)
{
    isect._coords = Vec3.Zero;
    double w0, w1, w2, w3;
    VertexWeights(isect, orgUp, dstUp, out w0, out w1);
    VertexWeights(isect, orgLo, dstLo, out w2, out w3);

    if (_combineCallback != null)
    {
        isect._data = _combineCallback(
            isect._coords,
            new[] { orgUp._data, dstUp._data, orgLo._data, dstLo._data },
            new[] { w0, w1, w2, w3 }
        );
    }
    else  // fix
    {
        isect._data = -1; 
    }
}

we’d solve the issue without needing the callback.

That’s what it seems to be to me. I might be wrong, of course, since it’s a complex piece of code and I’m still investigating because I also need this fix.
Sorry for the long message, but I hope I’ve at least helped contribute to investigating in the right direction.

[andyward]

This still feels like a bit of a post-fix to the root cause. Now all vertexes will be re-added/rebuilt after the first pass.
I think what I'd do is understand why isect._data has the value it has for the problem faces. What you're doing by setting _data to -1 is flagging all vertices to be re-added in TriangulateFaces. It may fix the issue but it's double handling every mesh.

Hopefully Ibrahim can chip in when back as he's more familiar with the Tessellator code.