Skip to content

Fix LibTess tessellation NRE and post-union vertex welding#1106

Merged
jamesbradleym merged 3 commits into
masterfrom
fix/csg-tessellation-fixes
May 26, 2026
Merged

Fix LibTess tessellation NRE and post-union vertex welding#1106
jamesbradleym merged 3 commits into
masterfrom
fix/csg-tessellation-fixes

Conversation

@jamesbradleym

@jamesbradleym jamesbradleym commented May 21, 2026

Copy link
Copy Markdown
Contributor

BACKGROUND:
Same MEP rendering failure as the Csg PR. Investigation surfaced two distinct Elements-side bugs in the tessellation → GLB pipeline:

  • LibTess NRE. When LibTess synthesizes new ContourVertexs at contour T-junctions or self-intersections (e.g., for a self-intersecting polygon coming from CSG output), those vertices have Data == null by default. The pack code (Tessellation.PackTessellationsIntoBuffers) unboxed v.Data to a 4-tuple shape without null-handling — instant NullReferenceException.

  • Post-union vertex welding. After a CSG boolean, polygons can carry duplicate Shared.Tag values across unrelated faces. The pack's vertex dedup map keyed on (tag, faceId, solidId) would weld vertices at different 3D positions into one index, producing corrupt mesh output (wrong normals, dropped triangles).

Both bugs fire in the same MEP geometry path (multi-solid Union → tessellate → GLB).

DESCRIPTION:

  • New HyparTessCombine class providing two CombineCallback factories — HyparDataCombine (4-tuple data shape for CSG/solid-face adapters) and CsgTexTagCombine ((Csg.Vector2D, int) for the legacy AddToMesh path). Each assigns unique synthetic tags via Interlocked.Increment from disjoint starting ranges.
  • Both CsgPolygonTessAdapter and SolidFaceTessAdapter now pass HyparDataCombine into tess.Tessellate(...), eliminating the null-Data NRE at the source.
    Tessellation.PackTessellationsIntoBuffers hardened with:
  • Pattern-match fallback for missing/malformed Data (synthetic tag, basis-vector UV — defense-in-depth even with callbacks wired).
  • Position-matched vertex reuse: on dedup-map collision, only weld when positions actually match; otherwise allocate a fresh synthetic tag and create a new vertex.
  • AddToMesh (the >4-vertex polygon branch) wires CsgTexTagCombine and null-safely unboxes triangle data via a new TexTagOrDefault helper.
  • New CsgVertexData struct replaces the spread (UV, uint, uint, uint) ValueTuple across 6+ producer/consumer sites.
  • Elements/lib/Csg.dll rebuilt from the companion Csg commit (Center geometry at origin during Union to preserve BSP precision at large coordinates Csg#2).
  • 7 new tests in CsgTests.cs: 5 proving tests (each fails when its target change is reverted) and 3 canary probes that assert real CSG Union output doesn't produce vertex tag collisions across positions.
  • 2 new MEP smoke tests in Elements.MEP/test/SerializationTests.cs — assert Tree and RoofDrain produce non-empty GLB without needing SkipCSGUnion = true.

TESTING:

  • dotnet test Elements/test/Elements.Tests.csproj --filter "FullyQualifiedName~Csg" — 22 tests pass.
  • dotnet test Elements.MEP/test/Elements.MEP.Tests.csproj --filter "FullyQualifiedName~UnionTessellates" — 2 smoke tests pass.
  • run any Hypar function that produces an MEP element (Tree or RoofDrain) at survey coordinates (e.g., (−49256, 12000, 5)). Before this PR: GLB output corrupt or missing geometry. After: renders correctly.
  • To verify any of the 5 proving tests catches a real regression: revert its target change and re-run the test — it must fail.

Tested with an Aliaxis workflow + function referencing local Elements:
Example workflow: workflows/edcd98fa-06bd-4a55-8e3d-f9ed103f3b1d
Example function: https://github.com/hypar-io/aliaxis/tree/main/ImportedSystemConfiguration (remember to set stream model to false when using hypar run)

FUTURE WORK:

  • Is there any future work needed or anticipated? Does this PR create any obvious technical debt?

REQUIRED:

  • All changes are up to date in CHANGELOG.md.

COMMENTS:

  • Any other notes.

BEFORE:
Screenshot 2026-05-21 at 11 30 19 AM
Screenshot 2026-05-21 at 11 30 01 AM

AFTER:
Screenshot 2026-05-21 at 11 30 58 AM
Screenshot 2026-05-21 at 11 30 48 AM


This change is Reviewable

@jamesbradleym
jamesbradleym requested review from a team and serenayl and removed request for a team May 21, 2026 18:24
@jamesbradleym
jamesbradleym marked this pull request as ready for review May 21, 2026 18:24

@serenayl serenayl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@serenayl reviewed 13 files and all commit messages, and made 2 comments.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on jamesbradleym).


Elements/src/Geometry/Tessellation/HyparTessCombine.cs line 24 at r1 (raw file):

    /// collide with the small, sequential tags emitted by the CSG library.
    /// </summary>
    internal static class HyparTessCombine

Nit: Should this just be something like CombineCallbacks instead of sounding extra-hypar-specific?

Code quote:

HyparTessCombine

Elements/src/Geometry/Tessellation/HyparTessCombine.cs line 31 at r1 (raw file):

        private static long _syntheticCsgTagCounter = 0x90000000L;

        internal static CombineCallback HyparDataCombine { get; } = Combine;

Nit: Does this maybe just want to be DataCombine? Should we name the private field it gets more consistently too, if there are two private combine fields?

Code quote:

HyparDataCombine

@jamesbradleym jamesbradleym left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jamesbradleym resolved 2 discussions.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained.

@jamesbradleym
jamesbradleym merged commit d4e7a95 into master May 26, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants